* Re: [PATCH v6 05/19] watchdog: orion: Make sure the watchdog is initially stopped
From: linux-0h96xk9xTtrk1uMJSBkQmQ @ 2014-02-10 16:48 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Jason Gunthorpe, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
Jason Cooper, Thomas Petazzoni, Gregory Clement, Lior Amsalem,
Sebastian Hesselbarth, Andrew Lunn
In-Reply-To: <20140210122223.GF10872@localhost>
Quoting Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> On Fri, Feb 07, 2014 at 10:43:14AM -0700, Jason Gunthorpe wrote:
>> On Fri, Feb 07, 2014 at 07:40:45AM -0300, Ezequiel Garcia wrote:
>>
>> > Well, this is related to the discussion about the bootloader not
>> > reseting the watchdog properly, provoking spurious watchdog triggering.
>> >
>> > Jason Gunthorpe explained [1] that we needed a particular sequence:
>> >
>> > 1. Disable WDT
>> > 2. Clear bridge
>> > 3. Enable WDT
>> >
>> > We added the irq handling to satisfy (2), and the watchdog stop for (1).
>>
>> The issue here is the driver configures two 'machine kill' elements:
>> the PANIC IRQ and the RstOut setup.
>>
>> Before configuring either of those the driver needs to ensure that any
>> old watchdog events are cleared out of the HW. We must not get a
>> spurious event.
>>
>> I agree not disabling an already functional and properly configured
>> counter from the bootloader is desirable.
>>
>> So lets break it down a bit..
>>
>> 1) The IRQ:
>> It looks like the cause bit latches high on watchdog timer
>> expiration but has no side effect unless it is unmasked.
>>
>> The new IRQ flow code ensures the bit is cleared during request_irq
>> so no old events can trigger the IRQ. Thus it is solved now.
>>
>
> Agreed.
>
>> 3) The timer itself:
>> The WDT is just a general timer with an optional hookup to the
>> rst control. If it is harmlessly counting but not resetting we need
>> to stop that before enabling rst out.
>>
>
> Actually, the current flow is to:
>
> 1. Disable rst out and then disable the counter, in probe().
>
> 2. Enable the counter, and then enable rst out, in start().
>
>> So, how about this for psuedo-code in probe:
>>
>> if (readl(RSTOUTn) & WDRstOutEn)
>> {
>> /* Watchdog is configured and may be down counting,
>> don't touch it */
>> request_irq(..);
>> }
>> else
>> {
>> /* Watchdog is not configured, fully disable the timer
>> and configure for watchdog operation. */
>> disable_watchdog();
>> request_irq();
>> writel(RSTOUTn), .. WDRstOutEn);
>> }
>>
>
> Sounds good, although it seems to me it's actually simpler:
>
> /* Let's make sure the watchdog is fully stopped, unless
> * it's explicitly enabled and running
> */
> if ( !(wdt_rst_out_en && wdt_timer_enabled) ) {
> watchdog_stop();
> }
>
if (!wdt_rst_out_en || !wdt_timer_enabled)
watchdog_stop();
seems to be a bit easier to understand.
Also watch out for coding style issues. Above code
snippet has multiple violations ;-).
Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 05/19] watchdog: orion: Make sure the watchdog is initially stopped
From: Ezequiel Garcia @ 2014-02-10 16:54 UTC (permalink / raw)
To: linux-0h96xk9xTtrk1uMJSBkQmQ
Cc: Jason Gunthorpe, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
Jason Cooper, Thomas Petazzoni, Gregory Clement, Lior Amsalem,
Sebastian Hesselbarth, Andrew Lunn
In-Reply-To: <20140210104850.48324szjwjihb7cw-S15kz1ZIOvSoVoj5zvVkGw@public.gmane.org>
On Mon, Feb 10, 2014 at 10:48:50AM -0600, linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org wrote:
> Quoting Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>
> > On Fri, Feb 07, 2014 at 10:43:14AM -0700, Jason Gunthorpe wrote:
> >> On Fri, Feb 07, 2014 at 07:40:45AM -0300, Ezequiel Garcia wrote:
> >>
> >> > Well, this is related to the discussion about the bootloader not
> >> > reseting the watchdog properly, provoking spurious watchdog triggering.
> >> >
> >> > Jason Gunthorpe explained [1] that we needed a particular sequence:
> >> >
> >> > 1. Disable WDT
> >> > 2. Clear bridge
> >> > 3. Enable WDT
> >> >
> >> > We added the irq handling to satisfy (2), and the watchdog stop for (1).
> >>
> >> The issue here is the driver configures two 'machine kill' elements:
> >> the PANIC IRQ and the RstOut setup.
> >>
> >> Before configuring either of those the driver needs to ensure that any
> >> old watchdog events are cleared out of the HW. We must not get a
> >> spurious event.
> >>
> >> I agree not disabling an already functional and properly configured
> >> counter from the bootloader is desirable.
> >>
> >> So lets break it down a bit..
> >>
> >> 1) The IRQ:
> >> It looks like the cause bit latches high on watchdog timer
> >> expiration but has no side effect unless it is unmasked.
> >>
> >> The new IRQ flow code ensures the bit is cleared during request_irq
> >> so no old events can trigger the IRQ. Thus it is solved now.
> >>
> >
> > Agreed.
> >
> >> 3) The timer itself:
> >> The WDT is just a general timer with an optional hookup to the
> >> rst control. If it is harmlessly counting but not resetting we need
> >> to stop that before enabling rst out.
> >>
> >
> > Actually, the current flow is to:
> >
> > 1. Disable rst out and then disable the counter, in probe().
> >
> > 2. Enable the counter, and then enable rst out, in start().
> >
> >> So, how about this for psuedo-code in probe:
> >>
> >> if (readl(RSTOUTn) & WDRstOutEn)
> >> {
> >> /* Watchdog is configured and may be down counting,
> >> don't touch it */
> >> request_irq(..);
> >> }
> >> else
> >> {
> >> /* Watchdog is not configured, fully disable the timer
> >> and configure for watchdog operation. */
> >> disable_watchdog();
> >> request_irq();
> >> writel(RSTOUTn), .. WDRstOutEn);
> >> }
> >>
> >
> > Sounds good, although it seems to me it's actually simpler:
> >
> > /* Let's make sure the watchdog is fully stopped, unless
> > * it's explicitly enabled and running
> > */
> > if ( !(wdt_rst_out_en && wdt_timer_enabled) ) {
> > watchdog_stop();
> > }
> >
>
> if (!wdt_rst_out_en || !wdt_timer_enabled)
> watchdog_stop();
>
> seems to be a bit easier to understand.
>
Yeah, I was actually planning to have a orion_wdt_enabled() function
to get the running and enabled status. Looks cleaner I think.
if (!orion_wdt_enabled())
watchdog_stop();
So the idea is OK?
I'll push the new series in a short while. Unfortunately, this change
means I have to rebase almost all the series, because I introduce the
orion watchdog struct :-(
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: Ivan T. Ivanov @ 2014-02-10 16:55 UTC (permalink / raw)
To: Andy Gross
Cc: Mark Brown, Grant Likely, Rob Herring, linux-spi, linux-arm-msm,
linux-kernel, devicetree, Alok Chauhan, Gilad Avidov, Kiran Gunda,
Sagar Dharia
In-Reply-To: <20140207173207.GA19974@qualcomm.com>
Hi Andy,
On Fri, 2014-02-07 at 11:32 -0600, Andy Gross wrote:
> On Fri, Feb 07, 2014 at 11:52:33AM +0200, Ivan T. Ivanov wrote:
> >
<snip>
> > > > +static int spi_qup_transfer_do(struct spi_qup *controller,
> > > > + struct spi_qup_device *chip,
> > > > + struct spi_transfer *xfer)
> > > > +{
> > > > + unsigned long timeout;
> > > > + int ret = -EIO;
> > > > +
> > > > + reinit_completion(&controller->done);
> > > > +
> > > > + timeout = DIV_ROUND_UP(controller->speed_hz, MSEC_PER_SEC);
> > > > + timeout = DIV_ROUND_UP(xfer->len * 8, timeout);
> > > > + timeout = 100 * msecs_to_jiffies(timeout);
> > > > +
> > > > + controller->rx_bytes = 0;
> > > > + controller->tx_bytes = 0;
> > > > + controller->error = 0;
> > > > + controller->xfer = xfer;
> > > > +
> > > > + if (spi_qup_set_state(controller, QUP_STATE_RUN)) {
> > > > + dev_warn(controller->dev, "cannot set RUN state\n");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + if (spi_qup_set_state(controller, QUP_STATE_PAUSE)) {
> > > > + dev_warn(controller->dev, "cannot set PAUSE state\n");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + spi_qup_fifo_write(controller, xfer);
> > > > +
> > > > + if (spi_qup_set_state(controller, QUP_STATE_RUN)) {
> > > > + dev_warn(controller->dev, "cannot set EXECUTE state\n");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + if (!wait_for_completion_timeout(&controller->done, timeout))
> > > > + ret = -ETIMEDOUT;
> > > > + else
> > > > + ret = controller->error;
> > > > +exit:
> > > > + controller->xfer = NULL;
> > >
> > > Should the manipulation of controller->xfer be protected by spinlock?
> >
> > :-). Probably. I am wondering, could I avoid locking if firstly place
> > QUP into RESET state and then access these field. This should stop
> > all activities in it, right?
>
> It's generally safest to not assume the hardware is going to do sane things.
> I'm concerned about spurious IRQs.
Ok, will add protection.
<snip>
> > > > +
> > > > + if (of_property_read_u32(dev->of_node, "spi-max-frequency", &max_freq))
> > > > + max_freq = 19200000;
> > >
> > > I'd set the default to 50MHz as that is the max supported by hardware. I'd just
> > > set max_freq declaration to 50MHz and then check the value if it is changed via
> > > DT.
> >
> > 50MHz doesn't seems to be supported on all chip sets. Currently common
> > denominator on all chip sets, that I can see, is 19.2MHz. I have tried
> > to test it with more than 19.2MHz on APQ8074 and it fails.
> >
>
> I guess my stance is to set it to the hardware max supported frequency if it is
> not specified. If that needs to be lower on a board because of whatever reason,
> they override it.
Ok, I will do in this way.
<snip>
> > >
> > > > +
> > > > + ret = clk_set_rate(cclk, max_freq);
> > > > + if (ret)
> > > > + dev_warn(dev, "fail to set SPI frequency %d\n", max_freq);
> > >
> > > Bail here?
> >
> > I don't know. What will be the consequences if controller continue to
> > operate on its default rate?
> >
>
> It is unclear. But if you can't set the rate that is configured or if there is
> a misconfiguration, it's probably better to exit the probe and catch it here.
My preference is to delay clock speed change till first
SPI transfer. And use wherever transfer itself mandate.
<snip>
> > > > +
> > > > +static int spi_qup_remove(struct platform_device *pdev)
> > > > +{
> > > > + struct spi_master *master = dev_get_drvdata(&pdev->dev);
> > > > + struct spi_qup *controller = spi_master_get_devdata(master);
> > > > +
> > > > + pm_runtime_get_sync(&pdev->dev);
> > > > +
> > >
> > > Do we need to wait for any current transactions to complete
> > > and do a devm_free_irq()?
> > >
> > > > + clk_disable_unprepare(controller->cclk);
> > > > + clk_disable_unprepare(controller->iclk);
> >
> > My understanding is:
> >
> > Disabling clocks will timeout transaction, if any. Core Device driver
> > will call: devm_spi_unregister(), which will wait pending transactions
> > to complete and then remove the SPI master.
>
> Disabling clocks will confuse the hardware. We cannot disable clocks while the
> spi core is active and transferring data.
I could follow approach taken by other SPI drivers, just reset
controller and disable clocks.
>
> >
> > > > +
> > > > + pm_runtime_put_noidle(&pdev->dev);
> > > > + pm_runtime_disable(&pdev->dev);
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static struct of_device_id spi_qup_dt_match[] = {
> > > > + { .compatible = "qcom,spi-qup-v2", },
> > >
> > > Need compatible tags of qcom,spi-qup-v2.1.1 (msm8974 v1) or qcom,spi-qup-v2.2.1
> > > (msm8974 v2)
> >
> > I am not aware of the difference. My board report v.20020000.
> > Is there difference of handling these controllers?
>
> There were some bug fixes between versions. None of those affect SPI (that I
> can tell), but it's better to be more descriptive and use the full versions in
> the compatible tags.
No strong preference here. Should I add qcom,spi-qup-v2.2.0, then? :-)
Regards,
Ivan
^ permalink raw reply
* Re: [PATCH v6 05/19] watchdog: orion: Make sure the watchdog is initially stopped
From: linux-0h96xk9xTtrk1uMJSBkQmQ @ 2014-02-10 16:57 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Jason Gunthorpe, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
Jason Cooper, Thomas Petazzoni, Gregory Clement, Lior Amsalem,
Sebastian Hesselbarth, Andrew Lunn
In-Reply-To: <20140210165405.GF15607@localhost>
Quoting Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> On Mon, Feb 10, 2014 at 10:48:50AM -0600, linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org wrote:
>> Quoting Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>>
>> > On Fri, Feb 07, 2014 at 10:43:14AM -0700, Jason Gunthorpe wrote:
>> >> On Fri, Feb 07, 2014 at 07:40:45AM -0300, Ezequiel Garcia wrote:
>> >>
>> >> > Well, this is related to the discussion about the bootloader not
>> >> > reseting the watchdog properly, provoking spurious watchdog triggering.
>> >> >
>> >> > Jason Gunthorpe explained [1] that we needed a particular sequence:
>> >> >
>> >> > 1. Disable WDT
>> >> > 2. Clear bridge
>> >> > 3. Enable WDT
>> >> >
>> >> > We added the irq handling to satisfy (2), and the watchdog
>> stop for (1).
>> >>
>> >> The issue here is the driver configures two 'machine kill' elements:
>> >> the PANIC IRQ and the RstOut setup.
>> >>
>> >> Before configuring either of those the driver needs to ensure that any
>> >> old watchdog events are cleared out of the HW. We must not get a
>> >> spurious event.
>> >>
>> >> I agree not disabling an already functional and properly configured
>> >> counter from the bootloader is desirable.
>> >>
>> >> So lets break it down a bit..
>> >>
>> >> 1) The IRQ:
>> >> It looks like the cause bit latches high on watchdog timer
>> >> expiration but has no side effect unless it is unmasked.
>> >>
>> >> The new IRQ flow code ensures the bit is cleared during request_irq
>> >> so no old events can trigger the IRQ. Thus it is solved now.
>> >>
>> >
>> > Agreed.
>> >
>> >> 3) The timer itself:
>> >> The WDT is just a general timer with an optional hookup to the
>> >> rst control. If it is harmlessly counting but not resetting we need
>> >> to stop that before enabling rst out.
>> >>
>> >
>> > Actually, the current flow is to:
>> >
>> > 1. Disable rst out and then disable the counter, in probe().
>> >
>> > 2. Enable the counter, and then enable rst out, in start().
>> >
>> >> So, how about this for psuedo-code in probe:
>> >>
>> >> if (readl(RSTOUTn) & WDRstOutEn)
>> >> {
>> >> /* Watchdog is configured and may be down counting,
>> >> don't touch it */
>> >> request_irq(..);
>> >> }
>> >> else
>> >> {
>> >> /* Watchdog is not configured, fully disable the timer
>> >> and configure for watchdog operation. */
>> >> disable_watchdog();
>> >> request_irq();
>> >> writel(RSTOUTn), .. WDRstOutEn);
>> >> }
>> >>
>> >
>> > Sounds good, although it seems to me it's actually simpler:
>> >
>> > /* Let's make sure the watchdog is fully stopped, unless
>> > * it's explicitly enabled and running
>> > */
>> > if ( !(wdt_rst_out_en && wdt_timer_enabled) ) {
>> > watchdog_stop();
>> > }
>> >
>>
>> if (!wdt_rst_out_en || !wdt_timer_enabled)
>> watchdog_stop();
>>
>> seems to be a bit easier to understand.
>>
>
> Yeah, I was actually planning to have a orion_wdt_enabled() function
> to get the running and enabled status. Looks cleaner I think.
>
> if (!orion_wdt_enabled())
> watchdog_stop();
>
> So the idea is OK?
>
Sounds good to me.
> I'll push the new series in a short while. Unfortunately, this change
> means I have to rebase almost all the series, because I introduce the
> orion watchdog struct :-(
Just trying to keep you busy :-)
Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: Mark Brown @ 2014-02-10 17:09 UTC (permalink / raw)
To: Ivan T. Ivanov
Cc: Grant Likely, Rob Herring, linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Alok Chauhan, Gilad Avidov,
Kiran Gunda, Sagar Dharia
In-Reply-To: <1392049766.2853.43.camel@iivanov-dev>
[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]
On Mon, Feb 10, 2014 at 06:29:26PM +0200, Ivan T. Ivanov wrote:
> On Fri, 2014-02-07 at 17:12 +0000, Mark Brown wrote:
> > > + if (!xfer)
> > > + return IRQ_HANDLED;
> > Are you sure? It seems wrong to just ignore interrupts, some comments
> > would help explain why.
> Of course this should never happen. This was left from initial stage
> of the implementation.
OK, so reporting them as errors seems better then.
> > > + if (controller->speed_hz != chip->speed_hz) {
> > > + ret = clk_set_rate(controller->cclk, chip->speed_hz);
> > > + if (ret) {
> > > + dev_err(controller->dev, "fail to set frequency %d",
> > > + chip->speed_hz);
> > > + return -EIO;
> > > + }
> > > + }
> > Is calling into the clock framework really so expensive that we need to
> > avoid doing it?
> Probably not. But why to call it if the frequency is the same.
It's less code that could go wrong and the check is already there in the
clock framework hopefully.
> > You also shouldn't be interacting with the hardware in
> > setup().
> This is internal hw-setup, not master::setup() or I am
> missing something?
The naming could probably be clearer then - config or something.
> > > + if (chip->bits_per_word <= 8)
> > > + controller->bytes_per_word = 1;
> > > + else if (chip->bits_per_word <= 16)
> > > + controller->bytes_per_word = 2;
> > > + else
> > > + controller->bytes_per_word = 4;
> > This looks like a switch statement, and looking at the above it's not
> > clear that the device actually supports anything other than whole bytes.
> > I'm not sure what that would mean from an API point of view.
> SPI API didn't validate struct spi_transfer::len field.
It's supposed to; if the validation is incomplete then that should be
fixed.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V3] net/dt: Add support for overriding phy configuration from device tree
From: Florian Fainelli @ 2014-02-10 17:09 UTC (permalink / raw)
To: Gerlando Falauto
Cc: Matthew Garrett, netdev,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Kishon Vijay Abraham I
In-Reply-To: <52F8FB03.6040606-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
Hi Gerlando,
Le lundi 10 février 2014, 17:14:59 Gerlando Falauto a écrit :
> Hi,
>
> I'm currently trying to fix an issue for which this patch provides a
> partial solution, so apologies in advance for jumping into the
> discussion for my own purposes...
>
> On 02/04/2014 09:39 PM, Florian Fainelli wrote:> 2014-01-17 Matthew
>
> Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>:
> >> Some hardware may be broken in interesting and board-specific ways, such
> >> that various bits of functionality don't work. This patch provides a
> >> mechanism for overriding mii registers during init based on the
>
> contents of
>
> >> the device tree data, allowing board-specific fixups without having to
> >> pollute generic code.
> >
> > It would be good to explain exactly how your hardware is broken
> > exactly. I really do not think that such a fine-grained setting where
> > you could disable, e.g: 100BaseT_Full, but allow 100BaseT_Half to
> > remain usable makes that much sense. In general, Gigabit might be
> > badly broken, but 100 and 10Mbits/sec should work fine. How about the
> > MASTER-SLAVE bit, is overriding it really required?
> >
> > Is not a PHY fixup registered for a specific OUI the solution you are
> > looking for? I am also concerned that this creates PHY troubleshooting
> > issues much harder to debug than before as we may have no idea about
> > how much information has been put in Device Tree to override that.
> >
> > Finally, how about making this more general just like the BCM87xx PHY
> > driver, which is supplied value/reg pairs directly? There are 16
> > common MII registers, and 16 others for vendor specific registers,
> > this is just covering for about 2% of the possible changes.
>
> Good point. That would easily help me with my current issue, which
> requires autoneg to be disabled to begin with (by clearing BMCR_ANENABLE
> from register 0).
Is there a point in time (e.g: after some specific initial configuration has
been made) where BMCR_ANENABLE can be used?
> This would not however fix it entirely (I tried a quick hardwired
> implementation), as the whole PHY machinery would not take that into
> account and would re-enable autoneg anyway.
> I also tried changing the patch so that phydev->support gets updated
There are multiple things that you could try doing here:
- override the PHY state machine in your read_status callback to make sure
that you always set phydev->autoneg set to AUTONEG_ENABLE
- clear the SUPPORTED_Autoneg bits from phydev->supported right after PHY
registration and before the call to phy_start()
- set the PHY_HAS_MAGICANEG bit in your PHY driver flag
>
> (instead of phydev->advertising):
> >> + if (!of_property_read_u32(np, override->prop, &tmp)) {
> >> + if (tmp) {
> >> + *val |= override->value;
> >> + phydev->advertising |=
>
> override->supported;
>
> >> + } else {
> >> + phydev->advertising &=
>
> ~(override->supported);
>
> >> + }
> >> +
> >> + *mask |= override->value;
>
> What I find weird is that the only way phydev->autoneg could ever be set
> to disabled is from here (phy.c):
>
> static void phy_sanitize_settings(struct phy_device *phydev)
> {
> u32 features = phydev->supported;
> int idx;
>
> /* Sanitize settings based on PHY capabilities */
> if ((features & SUPPORTED_Autoneg) == 0)
> phydev->autoneg = AUTONEG_DISABLE;
>
> which is in turn only called when phydev->autoneg is set to
> AUTONEG_DISABLE to begin with:
>
> int phy_start_aneg(struct phy_device *phydev)
> {
> int err;
>
> mutex_lock(&phydev->lock);
>
> if (AUTONEG_DISABLE == phydev->autoneg)
> phy_sanitize_settings(phydev);
>
> So could someone please help me figure out what I'm missing here?
At first glance it looks like the PHY driver should be reading the phydev-
>autoneg value when the PHY driver config_aneg() callback is called to be
allowed to set the forced speed and settings.
The way phy_sanitize_settings() is coded does not make it return a mask of
features, but only the forced supported speed and duplex. Then when the link
is forced but we are having some issues getting a link status, libphy tries
lower speeds and this function is used again to provide the next speed/duplex
pair to try.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: Ivan T. Ivanov @ 2014-02-10 17:11 UTC (permalink / raw)
To: dsneddon-sgV2jX0FEOL9JmXXK+q4OQ
Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
inux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, alokc-sgV2jX0FEOL9JmXXK+q4OQ,
gavidov-sgV2jX0FEOL9JmXXK+q4OQ, kgunda-sgV2jX0FEOL9JmXXK+q4OQ,
sdharia-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <3388d68dc907e9190d997fad95830d74.squirrel-mMfbam+mt9083fI46fginR2eb7JE58TQ@public.gmane.org>
Hi Dan,
On Mon, 2014-02-10 at 16:21 +0000, dsneddon-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org wrote:
> Hi Ivan,
> >
> >> > +
> >> > +static int spi_qup_set_state(struct spi_qup *controller, u32 state)
> >> > +{
> >> > + unsigned long loop = 0;
> >> > + u32 cur_state;
> >> > +
> >> > + cur_state = readl_relaxed(controller->base + QUP_STATE);
> >> Make sure the state is valid before you read the current state.
> >
> > Why? Controller is always left in valid state (after probe and every
> > transaction)? I know that CAF code contain this check, but now driver
> > is little bit different. I have made some tests and controller is
> > always in valid state in this point.
>
> The hardware programming guide we recommends doing this. I'd have to talk
> to the hardware designers to know exactly the reason why. I can follow up
> on this if you'd like.
>
Ok, thanks. I could add it back. Please, could you point me to place
in driver where this could happen.
> >
> >> > + /*
> >> > + * Per spec: for PAUSE_STATE to RESET_STATE, two writes
> >> > + * of (b10) are required
> >> > + */
> >> > + if (((cur_state & QUP_STATE_MASK) == QUP_STATE_PAUSE) &&
> >> > + (state == QUP_STATE_RESET)) {
> >> > + writel_relaxed(QUP_STATE_CLEAR, controller->base +
> >> > QUP_STATE);
> >> > + writel_relaxed(QUP_STATE_CLEAR, controller->base +
> >> > QUP_STATE);
> >> > + } else {
> >> Make sure you don't transition from RESET to PAUSE.
> >
> > I don't see this to be handled in CAF code. Could you give me
> > more details, please?
> >
> > Right now possible state transactions are:
> >
> > * if everything is fine:
> > RESET -> RUN -> PAUSE -> RUN -> RESET
> > * in case of error:
> > RESET -> RUN -> RESET
> > RESET -> RUN -> PAUSE -> RESET
> >
> > Please correct me if I am wrong.
>
> According to the hardware documentation if the hardware is in the RESET
> state and we try to transition to the PAUSE state the hardware behavior is
> "undefined", which usually means bad things will happen. Admittedly, if
> the driver always follows the "valid" rules (the ones you've listed above)
> it _should_ never happen. However, it is _possible_ the hardware is in
> the RESET state while the driver thinks it's in the RUN state and the
> driver tries to put is in the PAUSE state. In other words, I'd like to
> err on the side of caution since the check doesn't really cost us
> anything.
Ok that is fine, but did you see where/how this could happen in
the current implementation. If this could happen I will like to fix it.
>
> >
> >> > +
> >> > +static void spi_qup_fifo_read(struct spi_qup *controller,
> >> > + struct spi_transfer *xfer)
> >> > +{
> >> > + u8 *rx_buf = xfer->rx_buf;
> >> > + u32 word, state;
> >> > + int idx, shift;
> >> > +
> >> > + while (controller->rx_bytes < xfer->len) {
> >> > +
> >> > + state = readl_relaxed(controller->base +
> >> QUP_OPERATIONAL);
> >> > + if (0 == (state & QUP_OP_IN_FIFO_NOT_EMPTY))
> >> > + break;
> >> > +
> >> > + word = readl_relaxed(controller->base +
> >> QUP_INPUT_FIFO);
> >> > +
> >> > + for (idx = 0; idx < controller->bytes_per_word &&
> >> > + controller->rx_bytes < xfer->len; idx++,
> >> > + controller->rx_bytes++) {
> >> > +
> >> > + if (!rx_buf)
> >> > + continue;
> >> If there is no rx_buf just set rx_bytes to xfer->len and skip the loop
> >> entirely.
> >
> > Well, FIFO buffer still should be drained, right?
> > Anyway. I am looking for better way to handle this.
> > Same applies for filling FIFO buffer.
> >
>
> Yes. Still read from the FIFO but skip the for loop since we're just
> adding bytes_per_word to the total rx_byte count one iteration at a time
> and not doing anything with the data.
>
My point was: If I made rx_bytes equal xfer->len, outer while loop will
be terminated before FIFO was drained :-). I will see how to handle
this better.
Thanks,
Ivan
> Thanks,
> Dan
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 1/6] mailbox: add core framework
From: Courtney Cavin @ 2014-02-10 17:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: s-anna@ti.com, rob.herring@calxeda.com,
rafael.j.wysocki@intel.com, mark.langsdorf@calxeda.com,
tony@atomide.com, omar.ramirez@copitl.com,
gregkh@linuxfoundation.org, pawel.moll@arm.com,
mark.rutland@arm.com, ijc+devicetree@hellion.org.uk,
galak@codeaurora.org, rob@landley.net, linux-doc@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <4706525.lB7VmvWQMJ@wuerfel>
On Mon, Feb 10, 2014 at 03:11:00PM +0100, Arnd Bergmann wrote:
> On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> > The mailbox drivers are fragmented, and some implement their own core.
> > Unify the drivers and implement common functionality in a framework.
> >
> > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
>
> This seems pretty cool overall, great to see someone getting at it@
I'm glad to hear that there's some interest!
> > +static void of_mbox_adapter_add(struct mbox_adapter *adap)
> > +{
> > + if (!adap->dev)
> > + return;
> > +
> > + if (!adap->of_xlate) {
> > + adap->of_xlate = of_mbox_simple_xlate;
> > + adap->of_n_cells = 1;
> > + }
> > +
> > + of_node_get(adap->dev->of_node);
> > +}
>
> You should probably check if of_n_cells matches the device node
> #mbox-cells value, otherwise the xlate function will get confused.
Ok. I was under the impression that the adapter implementations would
add something like that, but I see no reason why putting it here would
hurt.
> > +
> > + mutex_lock(&mbox_lock);
> > + list_add(&adap->list, &mbox_adapters);
> > +
> > + of_mbox_adapter_add(adap);
> > + mutex_unlock(&mbox_lock);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(mbox_adapter_add);
>
> Please use EXPORT_SYMBOL_GPL here and elsewhere.
Ok.
> > +/**
> > + * mbox_channel_notify() - notify the core that a channel has a message
> > + * @chan: the channel which has data
> > + * @data: the location of said data
> > + * @len: the length of specified data
> > + *
> > + * This function may be called from interrupt/no-sleep context.
> > + */
> > +int mbox_channel_notify(struct mbox_channel *chan,
> > + const void *data, unsigned int len)
> > +{
> > + return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> > +}
> > +EXPORT_SYMBOL(mbox_channel_notify);
>
> What is the reason to use a notifier chain here? Isn't a simple
> callback function pointer enough? I would expect that each mailbox
> can have exactly one consumer, not multiple ones.
Mostly because I didn't see a reason not to. While a callback function
(and private data) would probably be sufficient, I don't see a specific
reason why a mailbox cannot have multiple consumers, and the API
currently is designed around that concept.
> > +/**
> > + * mbox_add_table() - add a lookup table for adapter consumers
> > + * @table: array of consumers to register
> > + * @num: number of consumers in array
> > + */
> > +void __init mbox_add_table(struct mbox_lookup *table, unsigned int num)
> > +{
> > + mutex_lock(&mbox_lookup_lock);
> > + while (num--) {
> > + if (table->provider && (table->dev_id || table->con_id))
> > + list_add_tail(&table->list, &mbox_lookup_list);
> > + table++;
> > + }
> > + mutex_unlock(&mbox_lookup_lock);
> > +}
> > +EXPORT_SYMBOL(mbox_add_table);
>
> I don't understand this part of the API. Why do you need a separate
> lookup table here? Isn't that what the DT lookup does already?
It is. The lookup/table stuff here is specifically for non-DT-based
mailboxes.
> > +/**
> > + * mbox_request() - lookup and request a MBOX channel
> > + * @dev: device for channel consumer
> > + * @con_id: consumer name
> > + * @nb: notifier block used for receiving messages
> > + *
> > + * The notifier is called as atomic on new messages, so you may not sleep
> > + * in the notifier callback function.
> > + */
> > +struct mbox *mbox_request(struct device *dev, const char *con_id,
> > + struct notifier_block *nb)
> > +{
> > + struct mbox_adapter *adap;
> > + struct mbox_channel *chan;
> > + struct mbox *mbox;
> > + int index = 0;
> > +
> > + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> > + return of_mbox_request(dev->of_node, con_id, nb);
>
> What use case do you have in mind for !CONFIG_OF?
None particularly, except for the existing implementations in
drivers/mailbox. I simply presumed it wouldn't hurt to implement lookup
tables similar to those the pwm core.
> > +/**
> > + * struct mbox_adapter_ops - MBOX adapter operations
> > + * @put_message: hook for putting messages in the channels MBOX
> > + * @request: optional hook for requesting an MBOX channel
> > + * @release: optional hook for releasing an MBOX channel
> > + * @owner: helps prevent removal of modules exporting active MBOX channels
> > + */
> > +struct mbox_adapter_ops {
> > + int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
> > + const void *, unsigned int);
> > + int (*request)(struct mbox_adapter *, struct mbox_channel *);
> > + int (*release)(struct mbox_adapter *, struct mbox_channel *);
> > + struct module *owner;
> > +};
>
> I think we will need a peek_message() callback for the upcoming
> QMTM driver, to allow client drivers to get a message out before
> the mailbox driver gets an IRQ. This will be used for IRQ mitigation
> in the network driver.
Eeek! I'm not very fond of 'peek' functions, but I guess I can see a
reason for IRQ mitigation here. I still cannot help but to try to think
my way out of implementing peek.
What would be the callback flow here? There's no guarantee that a
mailbox implementation isn't implemented over a sleepy bus, which would
render peek somewhat useless. Additionally, we have the adapter
protection mutex which can sleep anyway. This means that a consumer can
not call peek from anywhere atomic, including a notifier, which I think
is your use-case.
Perhaps a FEED_ME return from a notifier, requesting more 'mail' if
available?
>
> Arnd
Thanks for looking! I appreciate the feedback.
-Courtney
^ permalink raw reply
* [PATCH 0/6] Marvell Armada 375 and 38x clocks drivers
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
Hi Mike,
Here are patches that add the clocks drivers for two new Marvell ARM
SOCs that belong to the mach-mvebu family: the Armada 375 and the
Armada 380/385. They are based on Cortex-A9 CPU cores, and share a
number of peripherals with their predecessors in the mach-mvebu
family.
The drivers added are similar to the one already used for the other
SoCs of this family, as usual only the data are different the logic
remains the same ans allow us to use the common part.
The core support (arch/arm/mach-mvebu) for these SOCs have just been
posted, and we're aiming at having this merged for 3.15 if possible.
Thanks,
Gregory
Gregory CLEMENT (4):
clk: mvebu: add clock support for Armada 375
dt: Update binding information for mvebu core clocks with Armada 375
dt: Update binding information for mvebu gating clocks with Armada 375
clk: mvebu: add clock support for Armada 380/385
Thomas Petazzoni (2):
dt: Update binding information for mvebu core clocks with Armada
380/385
dt: Update binding information for mvebu gating clocks with Armada
380/385
.../devicetree/bindings/clock/mvebu-core-clock.txt | 14 ++
.../bindings/clock/mvebu-gated-clock.txt | 65 +++++++-
drivers/clk/mvebu/Kconfig | 8 +
drivers/clk/mvebu/Makefile | 2 +
drivers/clk/mvebu/armada-375.c | 184 +++++++++++++++++++++
drivers/clk/mvebu/armada-38x.c | 167 +++++++++++++++++++
6 files changed, 436 insertions(+), 4 deletions(-)
create mode 100644 drivers/clk/mvebu/armada-375.c
create mode 100644 drivers/clk/mvebu/armada-38x.c
--
1.8.1.2
^ permalink raw reply
* [PATCH 1/6] clk: mvebu: add clock support for Armada 375
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement@free-electrons.com>
Add the clock support for the new SoC Armada 375: core clocks and
gating clocks.
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/clk/mvebu/Kconfig | 4 +
drivers/clk/mvebu/Makefile | 1 +
drivers/clk/mvebu/armada-375.c | 184 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 189 insertions(+)
create mode 100644 drivers/clk/mvebu/armada-375.c
diff --git a/drivers/clk/mvebu/Kconfig b/drivers/clk/mvebu/Kconfig
index c339b829d3e3..a54ba170634b 100644
--- a/drivers/clk/mvebu/Kconfig
+++ b/drivers/clk/mvebu/Kconfig
@@ -13,6 +13,10 @@ config ARMADA_370_CLK
select MVEBU_CLK_CPU
select MVEBU_CLK_COREDIV
+config ARMADA_375_CLK
+ bool
+ select MVEBU_CLK_COMMON
+
config ARMADA_XP_CLK
bool
select MVEBU_CLK_COMMON
diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 21bbfb4a9f42..0b13811b9f62 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_MVEBU_CLK_CPU) += clk-cpu.o
obj-$(CONFIG_MVEBU_CLK_COREDIV) += clk-corediv.o
obj-$(CONFIG_ARMADA_370_CLK) += armada-370.o
+obj-$(CONFIG_ARMADA_375_CLK) += armada-375.o
obj-$(CONFIG_ARMADA_XP_CLK) += armada-xp.o
obj-$(CONFIG_DOVE_CLK) += dove.o
obj-$(CONFIG_KIRKWOOD_CLK) += kirkwood.o
diff --git a/drivers/clk/mvebu/armada-375.c b/drivers/clk/mvebu/armada-375.c
new file mode 100644
index 000000000000..c991a4d95e10
--- /dev/null
+++ b/drivers/clk/mvebu/armada-375.c
@@ -0,0 +1,184 @@
+/*
+ * Marvell Armada 375 SoC clocks
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ * Andrew Lunn <andrew@lunn.ch>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include "common.h"
+
+/*
+ * Core Clocks
+ */
+
+/*
+ * For the Armada 375 SoCs, the CPU, DDR and L2 clocks frequencies are
+ * all modified at the same time, and not separately as for the Armada
+ * 370 or the Armada XP SoCs.
+ *
+ * SAR0[21:17] : CPU frequency DDR frequency L2 frequency
+ * 6 = 400 MHz 400 MHz 200 MHz
+ * 15 = 600 MHz 600 MHz 300 MHz
+ * 21 = 800 MHz 534 MHz 400 MHz
+ * 25 = 1000 MHz 500 MHz 500 MHz
+ * others reserved.
+ *
+ * SAR0[22] : TCLK frequency
+ * 0 = 166 MHz
+ * 1 = 200 MHz
+ */
+
+#define SAR1_A375_TCLK_FREQ_OPT 22
+#define SAR1_A375_TCLK_FREQ_OPT_MASK 0x1
+#define SAR1_A375_CPU_DDR_L2_FREQ_OPT 17
+#define SAR1_A375_CPU_DDR_L2_FREQ_OPT_MASK 0x1F
+
+static const u32 armada_375_tclk_frequencies[] __initconst = {
+ 166000000,
+ 200000000,
+};
+
+static u32 __init armada_375_get_tclk_freq(void __iomem *sar)
+{
+ u8 tclk_freq_select;
+
+ tclk_freq_select = ((readl(sar) >> SAR1_A375_TCLK_FREQ_OPT) &
+ SAR1_A375_TCLK_FREQ_OPT_MASK);
+ return armada_375_tclk_frequencies[tclk_freq_select];
+}
+
+
+static const u32 armada_375_cpu_frequencies[] __initconst = {
+ 0, 0, 0, 0, 0, 0,
+ 400000000,
+ 0, 0, 0, 0, 0, 0, 0, 0,
+ 600000000,
+ 0, 0, 0, 0, 0,
+ 800000000,
+ 0, 0, 0,
+ 1000000000,
+};
+
+static u32 __init armada_375_get_cpu_freq(void __iomem *sar)
+{
+ u8 cpu_freq_select;
+
+ cpu_freq_select = ((readl(sar) >> SAR1_A375_CPU_DDR_L2_FREQ_OPT) &
+ SAR1_A375_CPU_DDR_L2_FREQ_OPT_MASK);
+ if (cpu_freq_select >= ARRAY_SIZE(armada_375_cpu_frequencies)) {
+ pr_err("Selected CPU frequency (%d) unsupported\n",
+ cpu_freq_select);
+ return 0;
+ } else
+ return armada_375_cpu_frequencies[cpu_freq_select];
+}
+
+enum { A375_CPU_TO_DDR, A375_CPU_TO_L2 };
+
+static const struct coreclk_ratio armada_375_coreclk_ratios[] __initconst = {
+ { .id = A375_CPU_TO_L2, .name = "l2clk" },
+ { .id = A375_CPU_TO_DDR, .name = "ddrclk" },
+};
+
+static const int armada_375_cpu_l2_ratios[32][2] __initconst = {
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {1, 2}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {1, 2},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {1, 2}, {0, 1}, {0, 1},
+ {0, 1}, {1, 2}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+};
+
+static const int armada_375_cpu_ddr_ratios[32][2] __initconst = {
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {1, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {2, 3},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {2, 3}, {0, 1}, {0, 1},
+ {0, 1}, {1, 2}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+};
+
+static void __init armada_375_get_clk_ratio(
+ void __iomem *sar, int id, int *mult, int *div)
+{
+ u32 opt = ((readl(sar) >> SAR1_A375_CPU_DDR_L2_FREQ_OPT) &
+ SAR1_A375_CPU_DDR_L2_FREQ_OPT_MASK);
+
+ switch (id) {
+ case A375_CPU_TO_L2:
+ *mult = armada_375_cpu_l2_ratios[opt][0];
+ *div = armada_375_cpu_l2_ratios[opt][1];
+ break;
+ case A375_CPU_TO_DDR:
+ *mult = armada_375_cpu_ddr_ratios[opt][0];
+ *div = armada_375_cpu_ddr_ratios[opt][1];
+ break;
+ }
+}
+
+static const struct coreclk_soc_desc armada_375_coreclks = {
+ .get_tclk_freq = armada_375_get_tclk_freq,
+ .get_cpu_freq = armada_375_get_cpu_freq,
+ .get_clk_ratio = armada_375_get_clk_ratio,
+ .ratios = armada_375_coreclk_ratios,
+ .num_ratios = ARRAY_SIZE(armada_375_coreclk_ratios),
+};
+
+static void __init armada_375_coreclk_init(struct device_node *np)
+{
+ mvebu_coreclk_setup(np, &armada_375_coreclks);
+}
+CLK_OF_DECLARE(armada_375_core_clk, "marvell,armada-375-core-clock",
+ armada_375_coreclk_init);
+
+/*
+ * Clock Gating Control
+ */
+static const struct clk_gating_soc_desc armada_375_gating_desc[] __initconst = {
+ { "mu", NULL, 2 },
+ { "pp", NULL, 3 },
+ { "ptp", NULL, 4 },
+ { "pex0", NULL, 5 },
+ { "pex1", NULL, 6 },
+ { "audio", NULL, 8 },
+ { "nd_clk", "nand", 11 },
+ { "sata0_link", "sata0_core", 14 },
+ { "sata0_core", NULL, 15 },
+ { "usb3", NULL, 16 },
+ { "sdio", NULL, 17 },
+ { "usb", NULL, 18 },
+ { "gop", NULL, 19 },
+ { "sata1_link", "sata1_core", 20 },
+ { "sata1_core", NULL, 21 },
+ { "xor0", NULL, 22 },
+ { "xor1", NULL, 23 },
+ { "copro", NULL, 24 },
+ { "tdm", NULL, 25 },
+ { "crypto0_enc", NULL, 28 },
+ { "crypto0_core", NULL, 29 },
+ { "crypto1_enc", NULL, 30 },
+ { "crypto1_core", NULL, 31 },
+ { }
+};
+
+static void __init armada_375_clk_gating_init(struct device_node *np)
+{
+ mvebu_clk_gating_setup(np, armada_375_gating_desc);
+}
+CLK_OF_DECLARE(armada_375_clk_gating, "marvell,armada-375-gating-clock",
+ armada_375_clk_gating_init);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/6] dt: Update binding information for mvebu core clocks with Armada 375
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement@free-electrons.com>
Add the binding information for the core clocks of the Armada 375 SoCs
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Documentation/devicetree/bindings/clock/mvebu-core-clock.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 1e662948661e..62fc34a1506d 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -11,6 +11,12 @@ The following is a list of provided IDs and clock names on Armada 370/XP:
3 = hclk (DRAM control clock)
4 = dramclk (DDR clock)
+The following is a list of provided IDs and clock names on Armada 375:
+ 0 = tclk (Internal Bus clock)
+ 1 = cpuclk (CPU clock)
+ 2 = l2clk (L2 Cache clock)
+ 3 = ddrclk (DDR clock)
+
The following is a list of provided IDs and clock names on Kirkwood and Dove:
0 = tclk (Internal Bus clock)
1 = cpuclk (CPU0 clock)
@@ -20,6 +26,7 @@ The following is a list of provided IDs and clock names on Kirkwood and Dove:
Required properties:
- compatible : shall be one of the following:
"marvell,armada-370-core-clock" - For Armada 370 SoC core clocks
+ "marvell,armada-375-core-clock" - For Armada 375 SoC core clocks
"marvell,armada-xp-core-clock" - For Armada XP SoC core clocks
"marvell,dove-core-clock" - for Dove SoC core clocks
"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
--
1.8.1.2
^ permalink raw reply related
* [PATCH 3/6] dt: Update binding information for mvebu gating clocks with Armada 375
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Add the binding information for the gating clocks of the Armada 375 SoCs
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
.../bindings/clock/mvebu-gated-clock.txt | 31 +++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
index fc2910fa7e45..cbecec1f64fa 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
@@ -1,6 +1,6 @@
* Gated Clock bindings for Marvell EBU SoCs
-Marvell Armada 370/XP, Dove and Kirkwood allow some peripheral clocks to be
+Marvell Armada 370/375/XP, Dove and Kirkwood allow some peripheral clocks to be
gated to save some power. The clock consumer should specify the desired clock
by having the clock ID in its "clocks" phandle cell. The clock ID is directly
mapped to the corresponding clock gating control bit in HW to ease manual clock
@@ -22,6 +22,34 @@ ID Clock Peripheral
28 ddr DDR Cntrl
30 sata1 SATA Host 0
+The following is a list of provided IDs for Armada 375:
+ID Clock Peripheral
+-----------------------------------
+2 mu Management Unit
+3 pp Packet Processor
+4 ptp PTP
+5 pex0 PCIe 0 Clock out
+6 pex1 PCIe 1 Clock out
+8 audio Audio Cntrl
+11 nd_clk Nand Flash Cntrl
+14 sata0_link SATA 0 Link
+15 sata0_core SATA 0 Core
+16 usb3 USB3 Host
+17 sdio SDHCI Host
+18 usb USB Host
+19 gop Gigabit Ethernet MAC
+20 sata1_link SATA 1 Link
+21 sata1_core SATA 1 Core
+22 xor0 XOR DMA 0
+23 xor1 XOR DMA 0
+24 copro Coprocessor
+25 tdm Time Division Mplx
+28 crypto0_enc Cryptographic Unit Port 0 Encryption
+29 crypto0_core Cryptographic Unit Port 0 Core
+30 crypto1_enc Cryptographic Unit Port 1 Encryption
+31 crypto1_core Cryptographic Unit Port 1 Core
+
+
The following is a list of provided IDs for Armada XP:
ID Clock Peripheral
-----------------------------------
@@ -95,6 +123,7 @@ ID Clock Peripheral
Required properties:
- compatible : shall be one of the following:
"marvell,armada-370-gating-clock" - for Armada 370 SoC clock gating
+ "marvell,armada-375-gating-clock" - for Armada 375 SoC clock gating
"marvell,armada-xp-gating-clock" - for Armada XP SoC clock gating
"marvell,dove-gating-clock" - for Dove SoC clock gating
"marvell,kirkwood-gating-clock" - for Kirkwood SoC clock gating
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 4/6] clk: mvebu: add clock support for Armada 380/385
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement@free-electrons.com>
Add the clock support for the new SoCs Armada 380 and Armada 385:
core clocks and gating clocks.
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/clk/mvebu/Kconfig | 4 +
drivers/clk/mvebu/Makefile | 1 +
drivers/clk/mvebu/armada-38x.c | 167 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 172 insertions(+)
create mode 100644 drivers/clk/mvebu/armada-38x.c
diff --git a/drivers/clk/mvebu/Kconfig b/drivers/clk/mvebu/Kconfig
index a54ba170634b..693f7be129f1 100644
--- a/drivers/clk/mvebu/Kconfig
+++ b/drivers/clk/mvebu/Kconfig
@@ -17,6 +17,10 @@ config ARMADA_375_CLK
bool
select MVEBU_CLK_COMMON
+config ARMADA_38X_CLK
+ bool
+ select MVEBU_CLK_COMMON
+
config ARMADA_XP_CLK
bool
select MVEBU_CLK_COMMON
diff --git a/drivers/clk/mvebu/Makefile b/drivers/clk/mvebu/Makefile
index 0b13811b9f62..4c66162fb0b4 100644
--- a/drivers/clk/mvebu/Makefile
+++ b/drivers/clk/mvebu/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_MVEBU_CLK_COREDIV) += clk-corediv.o
obj-$(CONFIG_ARMADA_370_CLK) += armada-370.o
obj-$(CONFIG_ARMADA_375_CLK) += armada-375.o
+obj-$(CONFIG_ARMADA_38X_CLK) += armada-38x.o
obj-$(CONFIG_ARMADA_XP_CLK) += armada-xp.o
obj-$(CONFIG_DOVE_CLK) += dove.o
obj-$(CONFIG_KIRKWOOD_CLK) += kirkwood.o
diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
new file mode 100644
index 000000000000..8bccf4ecdab6
--- /dev/null
+++ b/drivers/clk/mvebu/armada-38x.c
@@ -0,0 +1,167 @@
+/*
+ * Marvell Armada 380/385 SoC clocks
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ * Andrew Lunn <andrew@lunn.ch>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include "common.h"
+
+/*
+ * SAR[14:10] : Ratios between PCLK0, NBCLK, HCLK and DRAM clocks
+ *
+ * SAR[15] : TCLK frequency
+ * 0 = 250 MHz
+ * 1 = 200 MHz
+ */
+
+#define SAR_A380_TCLK_FREQ_OPT 15
+#define SAR_A380_TCLK_FREQ_OPT_MASK 0x1
+#define SAR_A380_CPU_DDR_L2_FREQ_OPT 10
+#define SAR_A380_CPU_DDR_L2_FREQ_OPT_MASK 0x1F
+
+static const u32 armada_38x_tclk_frequencies[] __initconst = {
+ 250000000,
+ 200000000,
+};
+
+static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
+{
+ u8 tclk_freq_select;
+
+ tclk_freq_select = ((readl(sar) >> SAR_A380_TCLK_FREQ_OPT) &
+ SAR_A380_TCLK_FREQ_OPT_MASK);
+ return armada_38x_tclk_frequencies[tclk_freq_select];
+}
+
+static const u32 armada_38x_cpu_frequencies[] __initconst = {
+ 0, 0, 0, 0,
+ 1066 * 1000 * 1000, 0, 0, 0,
+ 1332 * 1000 * 1000, 0, 0, 0,
+ 1600 * 1000 * 1000,
+};
+
+static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
+{
+ u8 cpu_freq_select;
+
+ cpu_freq_select = ((readl(sar) >> SAR_A380_CPU_DDR_L2_FREQ_OPT) &
+ SAR_A380_CPU_DDR_L2_FREQ_OPT_MASK);
+ if (cpu_freq_select >= ARRAY_SIZE(armada_38x_cpu_frequencies)) {
+ pr_err("Selected CPU frequency (%d) unsupported\n",
+ cpu_freq_select);
+ return 0;
+ }
+
+ return armada_38x_cpu_frequencies[cpu_freq_select];
+}
+
+enum { A380_CPU_TO_DDR, A380_CPU_TO_L2 };
+
+static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
+ { .id = A380_CPU_TO_L2, .name = "l2clk" },
+ { .id = A380_CPU_TO_DDR, .name = "ddrclk" },
+};
+
+static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+};
+
+static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+ {0, 1}, {0, 1}, {0, 1}, {0, 1},
+};
+
+static void __init armada_38x_get_clk_ratio(
+ void __iomem *sar, int id, int *mult, int *div)
+{
+ u32 opt = ((readl(sar) >> SAR_A380_CPU_DDR_L2_FREQ_OPT) &
+ SAR_A380_CPU_DDR_L2_FREQ_OPT_MASK);
+
+ switch (id) {
+ case A380_CPU_TO_L2:
+ *mult = armada_38x_cpu_l2_ratios[opt][0];
+ *div = armada_38x_cpu_l2_ratios[opt][1];
+ break;
+ case A380_CPU_TO_DDR:
+ *mult = armada_38x_cpu_ddr_ratios[opt][0];
+ *div = armada_38x_cpu_ddr_ratios[opt][1];
+ break;
+ }
+}
+
+static const struct coreclk_soc_desc armada_38x_coreclks = {
+ .get_tclk_freq = armada_38x_get_tclk_freq,
+ .get_cpu_freq = armada_38x_get_cpu_freq,
+ .get_clk_ratio = armada_38x_get_clk_ratio,
+ .ratios = armada_38x_coreclk_ratios,
+ .num_ratios = ARRAY_SIZE(armada_38x_coreclk_ratios),
+};
+
+static void __init armada_38x_coreclk_init(struct device_node *np)
+{
+ mvebu_coreclk_setup(np, &armada_38x_coreclks);
+}
+CLK_OF_DECLARE(armada_38x_core_clk, "marvell,armada-380-core-clock",
+ armada_38x_coreclk_init);
+
+/*
+ * Clock Gating Control
+ */
+static const struct clk_gating_soc_desc armada_38x_gating_desc[] __initconst = {
+ { "audio", NULL, 0 },
+ { "ge2", NULL, 2 },
+ { "ge1", NULL, 3 },
+ { "ge0", NULL, 4 },
+ { "pex1", NULL, 5 },
+ { "pex2", NULL, 6 },
+ { "pex3", NULL, 7 },
+ { "pex0", NULL, 8 },
+ { "usb3h0", NULL, 9 },
+ { "usb3h1", NULL, 10 },
+ { "usb3d", NULL, 11 },
+ { "bm", NULL, 13 },
+ { "crypto0z", NULL, 14 },
+ { "sata0", NULL, 15 },
+ { "crypto1z", NULL, 16 },
+ { "sdio", NULL, 17 },
+ { "usb2", NULL, 18 },
+ { "crypto1", NULL, 21 },
+ { "xor0", NULL, 22 },
+ { "crypto0", NULL, 23 },
+ { "tdm", NULL, 25 },
+ { "xor1", NULL, 28 },
+ { "sata1", NULL, 30 },
+ { }
+};
+
+static void __init armada_38x_clk_gating_init(struct device_node *np)
+{
+ mvebu_clk_gating_setup(np, armada_38x_gating_desc);
+}
+CLK_OF_DECLARE(armada_38x_clk_gating, "marvell,armada-380-gating-clock",
+ armada_38x_clk_gating_init);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 5/6] dt: Update binding information for mvebu core clocks with Armada 380/385
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Add the binding information for the core clocks of the Armada 380 and
Armada 385 SoCs
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
Documentation/devicetree/bindings/clock/mvebu-core-clock.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index 62fc34a1506d..307a503c5db8 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -17,6 +17,12 @@ The following is a list of provided IDs and clock names on Armada 375:
2 = l2clk (L2 Cache clock)
3 = ddrclk (DDR clock)
+The following is a list of provided IDs and clock names on Armada 380/385:
+ 0 = tclk (Internal Bus clock)
+ 1 = cpuclk (CPU clock)
+ 2 = l2clk (L2 Cache clock)
+ 3 = ddrclk (DDR clock)
+
The following is a list of provided IDs and clock names on Kirkwood and Dove:
0 = tclk (Internal Bus clock)
1 = cpuclk (CPU0 clock)
@@ -27,6 +33,7 @@ Required properties:
- compatible : shall be one of the following:
"marvell,armada-370-core-clock" - For Armada 370 SoC core clocks
"marvell,armada-375-core-clock" - For Armada 375 SoC core clocks
+ "marvell,armada-380-core-clock" - For Armada 380/385 SoC core clocks
"marvell,armada-xp-core-clock" - For Armada XP SoC core clocks
"marvell,dove-core-clock" - for Dove SoC core clocks
"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
--
1.8.1.2
^ permalink raw reply related
* [PATCH 6/6] dt: Update binding information for mvebu gating clocks with Armada 380/385
From: Gregory CLEMENT @ 2014-02-10 17:32 UTC (permalink / raw)
To: Mike Turquette
Cc: Thomas Petazzoni, Ezequiel Garcia, Sebastian Hesselbarth,
linux-arm-kernel, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
devicetree, linux-kernel, Gregory CLEMENT
In-Reply-To: <1392053569-28037-1-git-send-email-gregory.clement@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Add the binding information for the gating clocks of the Armada 380
SoCs and the Armada 385 SoCs.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
.../bindings/clock/mvebu-gated-clock.txt | 36 +++++++++++++++++++---
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
index cbecec1f64fa..1533e383d6e9 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
@@ -1,9 +1,10 @@
* Gated Clock bindings for Marvell EBU SoCs
-Marvell Armada 370/375/XP, Dove and Kirkwood allow some peripheral clocks to be
-gated to save some power. The clock consumer should specify the desired clock
-by having the clock ID in its "clocks" phandle cell. The clock ID is directly
-mapped to the corresponding clock gating control bit in HW to ease manual clock
+Marvell Armada 370/375/380/385/XP, Dove and Kirkwood allow some
+peripheral clocks to be gated to save some power. The clock consumer
+should specify the desired clock by having the clock ID in its
+"clocks" phandle cell. The clock ID is directly mapped to the
+corresponding clock gating control bit in HW to ease manual clock
lookup in datasheet.
The following is a list of provided IDs for Armada 370:
@@ -49,6 +50,32 @@ ID Clock Peripheral
30 crypto1_enc Cryptographic Unit Port 1 Encryption
31 crypto1_core Cryptographic Unit Port 1 Core
+The following is a list of provided IDs for Armada 380/385:
+ID Clock Peripheral
+-----------------------------------
+0 audio Audio
+2 ge2 Gigabit Ethernet 2
+3 ge1 Gigabit Ethernet 1
+4 ge0 Gigabit Ethernet 0
+5 pex1 PCIe 1
+6 pex2 PCIe 2
+7 pex3 PCIe 3
+8 pex4 PCIe 0
+9 usb3h0 USB3 Host 0
+10 usb3h1 USB3 Host 1
+11 usb3d USB3 Device
+13 bm Buffer Management
+14 crypto0z Cryptographic 0 Z
+15 sata0 SATA 0
+16 crypto1z Cryptographic 1 Z
+17 sdio SDIO
+18 usb2 USB 2
+21 crypto1 Cryptographic 1
+22 xor0 XOR 0
+23 crypto0 Cryptographic 0
+25 tdm Time Division Multiplexing
+28 xor1 XOR 1
+30 sata1 SATA 1
The following is a list of provided IDs for Armada XP:
ID Clock Peripheral
@@ -124,6 +151,7 @@ Required properties:
- compatible : shall be one of the following:
"marvell,armada-370-gating-clock" - for Armada 370 SoC clock gating
"marvell,armada-375-gating-clock" - for Armada 375 SoC clock gating
+ "marvell,armada-380-gating-clock" - for Armada 380/385 SoC clock gating
"marvell,armada-xp-gating-clock" - for Armada XP SoC clock gating
"marvell,dove-gating-clock" - for Dove SoC clock gating
"marvell,kirkwood-gating-clock" - for Kirkwood SoC clock gating
--
1.8.1.2
^ permalink raw reply related
* Re: Fwd: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: dsneddon @ 2014-02-10 17:41 UTC (permalink / raw)
To: Ivan T. Ivanov
Cc: dsneddon, broonie, grant.likely, robh+dt, linux-spi,
linux-arm-msm, inux-kernel, devicetree, alokc, gavidov, kgunda,
sdharia
In-Reply-To: <1392052316.2853.67.camel@iivanov-dev>
Hi Ivan,
>> >> > +static int spi_qup_set_state(struct spi_qup *controller, u32
>> state)
>> >> > +{
>> >> > + unsigned long loop = 0;
>> >> > + u32 cur_state;
>> >> > +
>> >> > + cur_state = readl_relaxed(controller->base + QUP_STATE);
>> >> Make sure the state is valid before you read the current state.
>> >
>> > Why? Controller is always left in valid state (after probe and every
>> > transaction)? I know that CAF code contain this check, but now driver
>> > is little bit different. I have made some tests and controller is
>> > always in valid state in this point.
>>
>> The hardware programming guide we recommends doing this. I'd have to
>> talk
>> to the hardware designers to know exactly the reason why. I can follow
>> up
>> on this if you'd like.
>>
>
> Ok, thanks. I could add it back. Please, could you point me to place
> in driver where this could happen.
>
>
>> >
>> >> > + /*
>> >> > + * Per spec: for PAUSE_STATE to RESET_STATE, two writes
>> >> > + * of (b10) are required
>> >> > + */
>> >> > + if (((cur_state & QUP_STATE_MASK) == QUP_STATE_PAUSE) &&
>> >> > + (state == QUP_STATE_RESET)) {
>> >> > + writel_relaxed(QUP_STATE_CLEAR, controller->base +
>> >> > QUP_STATE);
>> >> > + writel_relaxed(QUP_STATE_CLEAR, controller->base +
>> >> > QUP_STATE);
>> >> > + } else {
>> >> Make sure you don't transition from RESET to PAUSE.
>> >
>> > I don't see this to be handled in CAF code. Could you give me
>> > more details, please?
>> >
>> > Right now possible state transactions are:
>> >
>> > * if everything is fine:
>> > RESET -> RUN -> PAUSE -> RUN -> RESET
>> > * in case of error:
>> > RESET -> RUN -> RESET
>> > RESET -> RUN -> PAUSE -> RESET
>> >
>> > Please correct me if I am wrong.
>>
>> According to the hardware documentation if the hardware is in the RESET
>> state and we try to transition to the PAUSE state the hardware behavior
>> is
>> "undefined", which usually means bad things will happen. Admittedly, if
>> the driver always follows the "valid" rules (the ones you've listed
>> above)
>> it _should_ never happen. However, it is _possible_ the hardware is in
>> the RESET state while the driver thinks it's in the RUN state and the
>> driver tries to put is in the PAUSE state. In other words, I'd like to
>> err on the side of caution since the check doesn't really cost us
>> anything.
>
> Ok that is fine, but did you see where/how this could happen in
> the current implementation. If this could happen I will like to fix it.
I don't see a problem in the current implementation that would cause you
to get in the RESET state without knowing it. It's just my paranoia
kicking in.
>
>>
>> >
>> >> > +
>> >> > +static void spi_qup_fifo_read(struct spi_qup *controller,
>> >> > + struct spi_transfer *xfer)
>> >> > +{
>> >> > + u8 *rx_buf = xfer->rx_buf;
>> >> > + u32 word, state;
>> >> > + int idx, shift;
>> >> > +
>> >> > + while (controller->rx_bytes < xfer->len) {
>> >> > +
>> >> > + state = readl_relaxed(controller->base +
>> >> QUP_OPERATIONAL);
>> >> > + if (0 == (state & QUP_OP_IN_FIFO_NOT_EMPTY))
>> >> > + break;
>> >> > +
>> >> > + word = readl_relaxed(controller->base +
>> >> QUP_INPUT_FIFO);
>> >> > +
>> >> > + for (idx = 0; idx < controller->bytes_per_word &&
>> >> > + controller->rx_bytes < xfer->len; idx++,
>> >> > + controller->rx_bytes++) {
>> >> > +
>> >> > + if (!rx_buf)
>> >> > + continue;
>> >> If there is no rx_buf just set rx_bytes to xfer->len and skip the
>> loop
>> >> entirely.
>> >
>> > Well, FIFO buffer still should be drained, right?
>> > Anyway. I am looking for better way to handle this.
>> > Same applies for filling FIFO buffer.
>> >
>>
>> Yes. Still read from the FIFO but skip the for loop since we're just
>> adding bytes_per_word to the total rx_byte count one iteration at a time
>> and not doing anything with the data.
>>
>
> My point was: If I made rx_bytes equal xfer->len, outer while loop will
> be terminated before FIFO was drained :-). I will see how to handle
> this better.
You're right! Had to read that snippet again. Though, I think we can
skip draining the FIFO by setting the NO_INPUT bit in the QUP_CONFIG
register (and the NO_OUTPUT for writes).
^ permalink raw reply
* Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: Andy Gross @ 2014-02-10 17:47 UTC (permalink / raw)
To: Ivan T. Ivanov
Cc: Mark Brown, Grant Likely, Rob Herring,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Alok Chauhan, Gilad Avidov,
Kiran Gunda, Sagar Dharia
In-Reply-To: <1392051302.2853.56.camel@iivanov-dev>
On Mon, Feb 10, 2014 at 06:55:02PM +0200, Ivan T. Ivanov wrote:
[....]
> > > > Bail here?
> > >
> > > I don't know. What will be the consequences if controller continue to
> > > operate on its default rate?
> > >
> >
> > It is unclear. But if you can't set the rate that is configured or if there is
> > a misconfiguration, it's probably better to exit the probe and catch it here.
>
>
> My preference is to delay clock speed change till first
> SPI transfer. And use wherever transfer itself mandate.
>
That works. My only concern is that it might be nice to catch a configuration
problem early rather than wait for the SPI transfer to fail continuously.
[....]
> > > My understanding is:
> > >
> > > Disabling clocks will timeout transaction, if any. Core Device driver
> > > will call: devm_spi_unregister(), which will wait pending transactions
> > > to complete and then remove the SPI master.
> >
> > Disabling clocks will confuse the hardware. We cannot disable clocks while the
> > spi core is active and transferring data.
>
> I could follow approach taken by other SPI drivers, just reset
> controller and disable clocks.
You have to wait until the hardware is in a sane state. For the QUP, that means
in a RUN/PAUSE/RESET state. It cannot be in transition when you cut the clocks.
The safest thing to do is to get the QUP into the RESET state and then cut the
clocks.
[.....]
> > > I am not aware of the difference. My board report v.20020000.
> > > Is there difference of handling these controllers?
> >
> > There were some bug fixes between versions. None of those affect SPI (that I
> > can tell), but it's better to be more descriptive and use the full versions in
> > the compatible tags.
>
> No strong preference here. Should I add qcom,spi-qup-v2.2.0, then? :-)
According to the documentation, there is no v2.2.0. It appears there is some
disconnect between the specific HW revision and the documentation. I'll see if
I can get some clarification from the hardware guys. For now, I think the 2.1.1
and 2.2.1 tags are fine.
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: regression(ti platforms): next-20140210 (ehci?)
From: Roger Quadros @ 2014-02-10 17:50 UTC (permalink / raw)
To: Nishanth Menon, linux-omap, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Balbi, Felipe
Cc: linux-next-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org
In-Reply-To: <52F8F77B.70605-l0cyMroinI0@public.gmane.org>
+devicetree
On 02/10/2014 05:59 PM, Nishanth Menon wrote:
> Hi,
>
> A quick note to report that I saw regression in today's next tag (logs
> indicate around EHCI) boot on various TI platforms:
>
> Note: crane and sdp2430 are not expected to pass with
> multi_v7_defconfig (note: omap2plus_defconfig boot seems to be sane
> but USB is disabled there)
>
> next-20140210-multi_v7_defconfig
> 1: am335x-evm: Boot PASS: http://slexy.org/raw/s2zYHdPb94
> 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2UChLyzSE
> 3: am3517-evm: Boot FAIL: http://slexy.org/raw/s20Br9XLO1
> around ehci
>
> 4: am37x-evm: Boot FAIL: http://slexy.org/raw/s20mVz9Wc7
> around ehci
>
> 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s2byveBYtT
> 6: BeagleBoard-XM: Boot FAIL: http://slexy.org/raw/s21sOgJNwK
> around ehci
>
> 7: BeagleBone-Black: Boot PASS: http://slexy.org/raw/s2ovVNAmO7
> 8: crane: No Image built - Missing platform support?:
> 9: dra7: Boot PASS: http://slexy.org/raw/s217qwaXsM
> 10: ldp: Boot FAIL: http://slexy.org/raw/s203IvjE23
> around ehci
>
> 11: PandaBoard-ES: Boot FAIL: http://slexy.org/raw/s2NvkRx2YJ
> around ehci
I think the problem is that ehci-platform driver gets loaded instead of ehci-omap.
In the DT node we have compatible ids for both. e.g. for omap4.dtsi
usbhsehci: ehci@4a064c00 {
compatible = "ti,ehci-omap", "usb-ehci";
reg = <0x4a064c00 0x400>;
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
};
Shouldn't ehci-omap driver be getting a higher priority than usb-ehci?
A quick fix would be to eliminate "usb-ehci" from the DT node of all failing platforms.
cheers,
-roger
>
> 12: sdp2430: Boot FAIL: expected (v6 platform)
> 13: sdp3430: Boot FAIL: http://slexy.org/raw/s21cwW7PqH
> around ehci
>
> 14: sdp4430: Boot PASS: http://slexy.org/raw/s2hL39Pyl9
> 15: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s20UsDeuVB
> TOTAL = 15 boards, Booted Boards = 7, No Boot boards = 8
>
> next-20140207-multi_v7_defconfig
> 1: am335x-evm: Boot PASS: http://slexy.org/raw/s2yo795okf
> 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2TfAOi6XP
> 3: am3517-evm: Boot PASS: http://slexy.org/raw/s21sKT3pFN
> 4: am37x-evm: Boot PASS: http://slexy.org/raw/s21nCiNjAR
> 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s21uEu69lC
> 6: BeagleBoard-XM: Boot PASS: http://slexy.org/raw/s21SklkJs7
> 7: BeagleBone-Black: Boot PASS: http://slexy.org/raw/s21aYZvPl7
> 8: crane: No Image built - Missing platform support?:
> 9: dra7: Boot PASS: http://slexy.org/raw/s20soGBbYz
> 10: ldp: Boot PASS: http://slexy.org/raw/s20lDIIwgN
> 11: PandaBoard-ES: Boot PASS: http://slexy.org/raw/s2a5NWPUtE
> 12: sdp2430: Boot FAIL: expected (v6 platform)
> 13: sdp3430: Boot PASS: http://slexy.org/raw/s2osagMVWZ
> 14: sdp4430: Boot PASS: http://slexy.org/raw/s2NxmpHFaW
> 15: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s2PMcXzAUP
> TOTAL = 15 boards, Booted Boards = 13, No Boot boards = 2
>
> in comparison:
> v3.14-rc2-multi_v7_defconfig
> 1: am335x-evm: Boot PASS: http://slexy.org/raw/s2NWCJQczI
> 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2566ZAl5d
> 3: am3517-evm: Boot PASS: http://slexy.org/raw/s2msKg3ZQ9
> 4: am37x-evm: Boot PASS: http://slexy.org/raw/s2898HemYQ
> 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s20ajDkVgM
> 6: BeagleBoard-XM: Boot PASS: http://slexy.org/raw/s20YmD8SSG
> 7: BeagleBone-Black: Boot PASS: http://slexy.org/raw/s2sXDV7x0T
> 8: crane: No Image built - Missing platform support?:
> 9: dra7: Boot PASS: http://slexy.org/raw/s21Zz8NJrj
> 10: ldp: Boot PASS: http://slexy.org/raw/s21NANMvTx
> 11: PandaBoard-ES: Boot PASS: http://slexy.org/raw/s20NER4paD
> 12: sdp2430: Boot FAIL: expected (v6 platform)
> 13: sdp3430: Boot PASS: http://slexy.org/raw/s2WCHUl033
> 14: sdp4430: Boot PASS: http://slexy.org/raw/s21ySru6J1
> 15: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s2kztuFoSu
> TOTAL = 15 boards, Booted Boards = 13, No Boot boards = 2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 1/6] mailbox: add core framework
From: Rob Herring @ 2014-02-10 17:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Courtney Cavin, s-anna, Rob Herring, Wysocki, Rafael J,
Mark Langsdorf, Tony Lindgren, omar.ramirez, Greg Kroah-Hartman,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
linux-doc@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <4706525.lB7VmvWQMJ@wuerfel>
On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
>> The mailbox drivers are fragmented, and some implement their own core.
>> Unify the drivers and implement common functionality in a framework.
>>
>> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
[snip]
>> +int mbox_channel_notify(struct mbox_channel *chan,
>> + const void *data, unsigned int len)
>> +{
>> + return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
>> +}
>> +EXPORT_SYMBOL(mbox_channel_notify);
>
> What is the reason to use a notifier chain here? Isn't a simple
> callback function pointer enough? I would expect that each mailbox
> can have exactly one consumer, not multiple ones.
It probably can be a callback, but there can be multiple consumers. It
was only a notifier on the pl320 as there was no framework at the time
and to avoid creating custom interfaces between drivers. On highbank
for example, we can asynchronously receive the events for temperature
change, power off, and reset. So either there needs to be an event
demux somewhere or callbacks have to return whether they handled an
event or not.
Rob
^ permalink raw reply
* Re: [PATCH 6/6] dt: Update binding information for mvebu gating clocks with Armada 380/385
From: Andrew Lunn @ 2014-02-10 17:53 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Mike Turquette, Thomas Petazzoni, Lior Amsalem, Tawfik Bayouk,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Nadav Haklai,
Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
In-Reply-To: <1392053569-28037-7-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> +The following is a list of provided IDs for Armada 380/385:
> +ID Clock Peripheral
> +-----------------------------------
> +0 audio Audio
> +2 ge2 Gigabit Ethernet 2
> +3 ge1 Gigabit Ethernet 1
> +4 ge0 Gigabit Ethernet 0
> +5 pex1 PCIe 1
> +6 pex2 PCIe 2
> +7 pex3 PCIe 3
> +8 pex4 PCIe 0
Is that last one a typo? It at least looks a bit odd.
Thanks
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 6/6] dt: Update binding information for mvebu gating clocks with Armada 380/385
From: Thomas Petazzoni @ 2014-02-10 17:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: Gregory CLEMENT, Mike Turquette, Lior Amsalem, Tawfik Bayouk,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Nadav Haklai,
Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
In-Reply-To: <20140210175350.GD9995-g2DYL2Zd6BY@public.gmane.org>
Dear Andrew Lunn,
On Mon, 10 Feb 2014 18:53:50 +0100, Andrew Lunn wrote:
> > +The following is a list of provided IDs for Armada 380/385:
> > +ID Clock Peripheral
> > +-----------------------------------
> > +0 audio Audio
> > +2 ge2 Gigabit Ethernet 2
> > +3 ge1 Gigabit Ethernet 1
> > +4 ge0 Gigabit Ethernet 0
> > +5 pex1 PCIe 1
> > +6 pex2 PCIe 2
> > +7 pex3 PCIe 3
> > +8 pex4 PCIe 0
>
> Is that last one a typo? It at least looks a bit odd.
Right, this should be:
8 pex0 PCIe 0
(just checked again in the datasheet)
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: regression(ti platforms): next-20140210 (ehci?)
From: Nishanth Menon @ 2014-02-10 18:02 UTC (permalink / raw)
To: Roger Quadros, linux-omap, linux-usb, Balbi, Felipe
Cc: linux-next, devicetree@vger.kernel.org, tony@atomide.com
In-Reply-To: <52F9117C.8000405@ti.com>
On 02/10/2014 11:50 AM, Roger Quadros wrote:
> +devicetree
>
[...]
> In the DT node we have compatible ids for both. e.g. for omap4.dtsi
>
> usbhsehci: ehci@4a064c00 {
> compatible = "ti,ehci-omap", "usb-ehci";
> reg = <0x4a064c00 0x400>;
> interrupt-parent = <&gic>;
> interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> Shouldn't ehci-omap driver be getting a higher priority than usb-ehci?
>
> A quick fix would be to eliminate "usb-ehci" from the DT node of all failing platforms.
If the driver is not compatible with "usb-ehci", not sure why do we
even state that in dts node?
--
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Tanmay Inamdar @ 2014-02-10 18:06 UTC (permalink / raw)
To: Tanmay Inamdar, Arnd Bergmann, devicetree@vger.kernel.org,
linaro-kernel, linux-pci, Will Deacon, LKML, Catalin Marinas,
Bjorn Helgaas, LAKML
In-Reply-To: <20140208142207.GR4993@e106497-lin.cambridge.arm.com>
On Sat, Feb 8, 2014 at 6:22 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> On Sat, Feb 08, 2014 at 12:21:56AM +0000, Tanmay Inamdar wrote:
>> On Thu, Feb 6, 2014 at 2:18 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > On Wed, Feb 05, 2014 at 10:26:27PM +0000, Tanmay Inamdar wrote:
>> >> Hello Liviu,
>> >>
>> >> I did not get the first email of this particular patch on any of
>> >> subscribed mailing lists (don't know why), hence replying here.
>> >
>> > Strange, it shows in the MARC and GMANE archive for linux-pci, probably
>> > a hickup on your receiving side?
>> >
>> >>
>> >> +struct pci_host_bridge *
>> >> +pci_host_bridge_of_init(struct device *parent, int busno, struct pci_ops *ops,
>> >> + void *host_data, struct list_head *resources)
>> >> +{
>> >> + struct pci_bus *root_bus;
>> >> + struct pci_host_bridge *bridge;
>> >> +
>> >> + /* first parse the host bridge bus ranges */
>> >> + if (pci_host_bridge_of_get_ranges(parent->of_node, resources))
>> >> + return NULL;
>> >> +
>> >> + /* then create the root bus */
>> >> + root_bus = pci_create_root_bus(parent, busno, ops, host_data, resources);
>> >> + if (!root_bus)
>> >> + return NULL;
>> >> +
>> >> + bridge = to_pci_host_bridge(root_bus->bridge);
>> >> +
>> >> + return bridge;
>> >> +}
>> >>
>> >> You are keeping the domain_nr inside pci_host_bridge structure. In
>> >> above API, domain_nr is required in 'pci_find_bus' function called
>> >> from 'pci_create_root_bus'. Since the bridge is allocated after
>> >> creating root bus, 'pci_find_bus' always gets domain_nr as 0. This
>> >> will cause problem for scanning multiple domains.
>> >
>> > Good catch. I was switching between creating a pci_controller in arch/arm64 and
>> > adding the needed bits in pci_host_bridge. After internal review I've decided to
>> > add the domain_nr to pci_host_bridge, but forgot to update the code everywhere.
>> >
>> > Thanks for reviewing this, will fix in v2.
>> >
>> > Do you find porting to the new API straight forward?
>>
>> It is quite straight forward for MEM regions but for IO regions it is
>> not. You always assume IO resource starting at 0x0. IMO, this will
>> cause problem for systems with multiple ports / IO windows. You can
>> take a look at 'drivers/pci/host/pcie-designware.c'.
>>
>> Also the manipulations of addresses for IO_RESOURCES can be dangerous.
>> It can make some value negative. For example if my PCI IO range starts
>> in 32 bit address range say 0x8000_0000 with CPU address
>> 0xb0_8000_0000, window->offset after manipulation will become
>> negative.
>>
>> Personally I would like to do get all the PCI and CPU addresses from
>> my device tree as is and do the 'pci_add_resource_offset'. This will
>> help me setup my regions correctly without any further arithmetic.
>> Please let me know what you think.
>
> Yes, that parsing code of ranges is incorrect in light of the current discussion. I
> will try to propose a fix in the v2 series.
Okay. Thanks.
>
> Regarding your PCI IO range: if your bus address starts at 0x8000_0000, would
> that not cause problems with devices that use less than 32bits for address
> decoding? It is a rather unusual setup, I believe the x86 world (even PCI spec?)
> mandates IO bus ranges in the first 16MB of address range?
>
Yes. It will cause problem with few devices. Anyways it was just an example.
> Best regards,
> Liviu
>
>>
>> >
>> > Best regards,
>> > Liviu
>> >
>> >>
>> >>
>> >> On Mon, Feb 3, 2014 at 10:46 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> > On Monday 03 February 2014 18:33:48 Liviu Dudau wrote:
>> >> >> +/**
>> >> >> + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from DT
>> >> >> + * @dev: device node of the host bridge having the range property
>> >> >> + * @resources: list where the range of resources will be added after DT parsing
>> >> >> + *
>> >> >> + * This function will parse the "ranges" property of a PCI host bridge device
>> >> >> + * node and setup the resource mapping based on its content. It is expected
>> >> >> + * that the property conforms with the Power ePAPR document.
>> >> >> + *
>> >> >> + * Each architecture will then apply their filtering based on the limitations
>> >> >> + * of each platform. One general restriction seems to be the number of IO space
>> >> >> + * ranges, the PCI framework makes intensive use of struct resource management,
>> >> >> + * and for IORESOURCE_IO types they can only be requested if they are contained
>> >> >> + * within the global ioport_resource, so that should be limited to one IO space
>> >> >> + * range.
>> >> >
>> >> > Actually we have quite a different set of restrictions around I/O space on ARM32
>> >> > at the moment: Each host bridge can have its own 64KB range in an arbitrary
>> >> > location on MMIO space, and the total must not exceed 2MB of I/O space.
>> >> >
>> >> >> + */
>> >> >> +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
>> >> >> + struct list_head *resources)
>> >> >> +{
>> >> >> + struct resource *res;
>> >> >> + struct of_pci_range range;
>> >> >> + struct of_pci_range_parser parser;
>> >> >> + int err;
>> >> >> +
>> >> >> + pr_info("PCI host bridge %s ranges:\n", dev->full_name);
>> >> >> +
>> >> >> + /* Check for ranges property */
>> >> >> + err = of_pci_range_parser_init(&parser, dev);
>> >> >> + if (err)
>> >> >> + return err;
>> >> >> +
>> >> >> + pr_debug("Parsing ranges property...\n");
>> >> >> + for_each_of_pci_range(&parser, &range) {
>> >> >> + /* Read next ranges element */
>> >> >> + pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
>> >> >> + range.pci_space, range.pci_addr);
>> >> >> + pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
>> >> >> + range.cpu_addr, range.size);
>> >> >> +
>> >> >> + /* If we failed translation or got a zero-sized region
>> >> >> + * (some FW try to feed us with non sensical zero sized regions
>> >> >> + * such as power3 which look like some kind of attempt
>> >> >> + * at exposing the VGA memory hole) then skip this range
>> >> >> + */
>> >> >> + if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
>> >> >> + continue;
>> >> >> +
>> >> >> + res = kzalloc(sizeof(struct resource), GFP_KERNEL);
>> >> >> + if (!res) {
>> >> >> + err = -ENOMEM;
>> >> >> + goto bridge_ranges_nomem;
>> >> >> + }
>> >> >> +
>> >> >> + of_pci_range_to_resource(&range, dev, res);
>> >> >> +
>> >> >> + pci_add_resource_offset(resources, res,
>> >> >> + range.cpu_addr - range.pci_addr);
>> >> >> + }
>> >> >
>> >> > I believe of_pci_range_to_resource() will return the MMIO aperture for the
>> >> > I/O space window here, which is not what you are supposed to pass into
>> >> > pci_add_resource_offset.
>> >> >
>> >> >> +EXPORT_SYMBOL(pci_host_bridge_of_init);
>> >> >
>> >> > EXPORT_SYMBOL_GPL
>> >> >
>> >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> >> >> index 6e34498..16febae 100644
>> >> >> --- a/drivers/pci/probe.c
>> >> >> +++ b/drivers/pci/probe.c
>> >> >> @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>> >> >> list_for_each_entry_safe(window, n, resources, list) {
>> >> >> list_move_tail(&window->list, &bridge->windows);
>> >> >> res = window->res;
>> >> >> + /*
>> >> >> + * IO resources are stored in the kernel with a CPU start
>> >> >> + * address of zero. Adjust the data accordingly and remember
>> >> >> + * the offset
>> >> >> + */
>> >> >> + if (resource_type(res) == IORESOURCE_IO) {
>> >> >> + bridge->io_offset = res->start;
>> >> >> + res->end -= res->start;
>> >> >> + window->offset -= res->start;
>> >> >> + res->start = 0;
>> >> >> + }
>> >> >> offset = window->offset;
>> >> >> if (res->flags & IORESOURCE_BUS)
>> >> >
>> >> > Won't this break all existing host bridges?
>> >> >
>> >> > Arnd
>> >> >
>> >> > _______________________________________________
>> >> > linux-arm-kernel mailing list
>> >> > linux-arm-kernel@lists.infradead.org
>> >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>> >>
>> >
>> > --
>> > ====================
>> > | I would like to |
>> > | fix the world, |
>> > | but they're not |
>> > | giving me the |
>> > \ source code! /
>> > ---------------
>> > ¯\_(ツ)_/¯
>> >
>> > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>> >
>> > ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
>> > ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> ====================
> | I would like to |
> | fix the world, |
> | but they're not |
> | giving me the |
> \ source code! /
> ---------------
> ¯\_(ツ)_/¯
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
> ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
> ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
>
^ permalink raw reply
* [PATCH] ARM: dts: OMAP2+: Fix boot with multi_v7_defconfig
From: Roger Quadros @ 2014-02-10 18:10 UTC (permalink / raw)
To: tony-4v6yS6AI5VpBDgjK7y7TUQ
Cc: bcousson-rdvid1DuHRBWk0Htik3J/w, balbi-l0cyMroinI0,
nm-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Roger Quadros
The OMAP EHCI controller is not compatible with the EHCI
platform HCD driver so don't claim that we are.
This fixes boot on OMAP platforms with CONFIG_USB_EHCI_HCD_PLATFORM=y
e.g. multi_v7_defconfig
Reported-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
---
arch/arm/boot/dts/omap3.dtsi | 2 +-
arch/arm/boot/dts/omap4.dtsi | 2 +-
arch/arm/boot/dts/omap5.dtsi | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index a5fc83b..6b5dbf8 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -641,7 +641,7 @@
};
usbhsehci: ehci@48064800 {
- compatible = "ti,ehci-omap", "usb-ehci";
+ compatible = "ti,ehci-omap";
reg = <0x48064800 0x400>;
interrupt-parent = <&intc>;
interrupts = <77>;
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index d3f8a6e..f5d754b 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -706,7 +706,7 @@
};
usbhsehci: ehci@4a064c00 {
- compatible = "ti,ehci-omap", "usb-ehci";
+ compatible = "ti,ehci-omap";
reg = <0x4a064c00 0x400>;
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index a72813a..42fcffd 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -784,7 +784,7 @@
};
usbhsehci: ehci@4a064c00 {
- compatible = "ti,ehci-omap", "usb-ehci";
+ compatible = "ti,ehci-omap";
reg = <0x4a064c00 0x400>;
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: regression(ti platforms): next-20140210 (ehci?)
From: Roger Quadros @ 2014-02-10 18:16 UTC (permalink / raw)
To: Nishanth Menon, linux-omap, linux-usb, Balbi, Felipe
Cc: linux-next, devicetree@vger.kernel.org, tony@atomide.com
In-Reply-To: <52F91451.8050802@ti.com>
On 02/10/2014 08:02 PM, Nishanth Menon wrote:
> On 02/10/2014 11:50 AM, Roger Quadros wrote:
>> +devicetree
>>
> [...]
>> In the DT node we have compatible ids for both. e.g. for omap4.dtsi
>>
>> usbhsehci: ehci@4a064c00 {
>> compatible = "ti,ehci-omap", "usb-ehci";
>> reg = <0x4a064c00 0x400>;
>> interrupt-parent = <&gic>;
>> interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> Shouldn't ehci-omap driver be getting a higher priority than usb-ehci?
>>
>> A quick fix would be to eliminate "usb-ehci" from the DT node of all failing platforms.
>
> If the driver is not compatible with "usb-ehci", not sure why do we
> even state that in dts node?
>
>
I'm not sure either. Let's get rid of it.
Patch to fix the reported issue.
http://article.gmane.org/gmane.linux.drivers.devicetree/61204
cheers,
-roger
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox