* [PATCH v3] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 23:03 UTC (permalink / raw)
To: Felipe Balbi
Cc: Mark Rutland, devicetree, John Youn, Greg Kroah-Hartman,
linux-usb, linux-kernel, Rob Herring, Christian Lamparter,
linuxppc-dev
From: Christian Lamparter <chunkeey@googlemail.com>
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")
but without any driver support as the dwc2 driver wasn't available at
that time.
Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
Sorry, the previous one broke compilation. This fixes it.
Regards,
John
v3 [johnyoun]:
* Fixed compilation issue
v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/params.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..a786256 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = NULL },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.0
^ permalink raw reply related
* [PATCH v3] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 23:03 UTC (permalink / raw)
To: Felipe Balbi
Cc: Mark Rutland, devicetree, John Youn, Greg Kroah-Hartman,
linux-usb, linux-kernel, Rob Herring, Christian Lamparter,
linuxppc-dev
From: Christian Lamparter <chunkeey@googlemail.com>
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")
but without any driver support as the dwc2 driver wasn't available at
that time.
Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
Sorry, the previous one broke compilation. This fixes it.
Regards,
John
v3 [johnyoun]:
* Fixed compilation issue
v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/params.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..a786256 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = NULL },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.0
^ permalink raw reply related
* [PATCH v3] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 23:03 UTC (permalink / raw)
To: Felipe Balbi
Cc: Christian Lamparter, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Rutland, Rob Herring,
Greg Kroah-Hartman, John Youn
From: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")
but without any driver support as the dwc2 driver wasn't available at
that time.
Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.
Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Sorry, the previous one broke compilation. This fixes it.
Regards,
John
v3 [johnyoun]:
* Fixed compilation issue
v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/params.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..a786256 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = NULL },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.0
--
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 related
* [PATCH v3] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 23:03 UTC (permalink / raw)
To: Felipe Balbi
Cc: Christian Lamparter, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Mark Rutland, Rob Herring,
Greg Kroah-Hartman, John Youn
From: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")
but without any driver support as the dwc2 driver wasn't available at
that time.
Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.
Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Sorry, the previous one broke compilation. This fixes it.
Regards,
John
v3 [johnyoun]:
* Fixed compilation issue
v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/params.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..a786256 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = NULL },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.0
--
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: [RESEND PATCH 2/2] PCI: rockchip: Add quirk to disable RC's ASPM L0s
From: Rob Herring @ 2016-11-15 22:26 UTC (permalink / raw)
To: Shawn Lin
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Wenrui Li,
linux-pci-u79uwXL29TY76Z2rM5mHXA, Jeffy Chen, Brian Norris,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Bjorn Helgaas
In-Reply-To: <1479096666-112668-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Mon, Nov 14, 2016 at 12:11:06PM +0800, Shawn Lin wrote:
> Rockchip's RC outputs 100MHz reference clock but there are
> two methods for PHY to generate it.
>
> (1)One of them is to use system PLL to generate 100MHz clock and
> the PHY will relock it and filter signal noise then outputs the
> reference clock.
>
> (2)Another way is to share Soc's 24MHZ crystal oscillator with
> PHY and force PHY's DLL to generate 100MHz internally.
>
> When using case(2), the exit from L0s doesn't work fine occasionally
> due to the broken design of RC receiver's logical circuit. So even if
> we use extended-synch, it still fails for PHY to relock the bits from
> FTS sometimes. This will hang the system.
>
> Maybe we could argue that why not use case(1) to avoid it? The reason
> is that as we could see the reference clock is derived from system PLL
> and the path from it to PHY isn't so clean which means there are some
> noise introduced by power-domain and other buses can't be filterd out
> by PHY and we could see noise from the frequency spectrum by oscilloscope.
> This makes the TX compatibility test a little difficult to pass the spec.
> So case(1) and case(2) are both used indeed now. If using case(2), we
> should disable RC's L0s support, and that is why we need this property to
> indicate this quirk.
Doesn't the driver know which case it is using? I don't see why you need
the quirk property.
>
> Also after checking quirk.c, I noticed there is already a quirk for
> disabling L0s unconditionally, quirk_disable_aspm_l0s. But obviously we
> shouldn't do that as mentioned above that case(1) could still works fine
> with L0s.
^ permalink raw reply
* Re: [PATCH 08/10] ASoC: sun4i-codec: Add support for H3 codec
From: Rob Herring @ 2016-11-15 22:23 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, Maxime Ripard, Lee Jones, Mark Rutland,
alsa-devel, linux-arm-kernel, linux-kernel, devicetree,
Mylene Josserand
In-Reply-To: <20161112064648.26779-9-wens@csie.org>
On Sat, Nov 12, 2016 at 02:46:46PM +0800, Chen-Yu Tsai wrote:
> The codec on the H3 is similar to the one found on the A31. One key
> difference is the analog path controls are routed through the PRCM
> block. This is supported by the sun8i-codec-analog driver, and tied
> into this codec driver with the audio card's aux_dev.
>
> In addition, the H3 has no HP (headphone) and HBIAS support, and no
> MIC3 input. The FIFO related registers are slightly rearranged.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> .../devicetree/bindings/sound/sun4i-codec.txt | 3 +
Acked-by: Rob Herring <robh@kernel.org>
> sound/soc/sunxi/sun4i-codec.c | 71 ++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
^ permalink raw reply
* Re: [PATCH 04/10] ASoC: sun4i-codec: Add support for A23 codec
From: Rob Herring @ 2016-11-15 22:23 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, Maxime Ripard, Lee Jones, Mark Rutland,
alsa-devel, linux-arm-kernel, linux-kernel, devicetree,
Mylene Josserand
In-Reply-To: <20161112064648.26779-5-wens@csie.org>
On Sat, Nov 12, 2016 at 02:46:42PM +0800, Chen-Yu Tsai wrote:
> The codec in the A23 is similar to the one found on the A31. One key
> difference is the analog path controls are routed through the PRCM
> block. This is supported by the sun8i-codec-analog driver, and tied
> into this codec driver with the audio card's aux_dev.
>
> In addition, the A23 does not have LINEOUT, and it does not support
> headset jack detection or buttons.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> .../devicetree/bindings/sound/sun4i-codec.txt | 11 ++-
Acked-by: Rob Herring <robh@kernel.org>
> sound/soc/sunxi/sun4i-codec.c | 108 +++++++++++++++++++++
> 2 files changed, 117 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH 01/10] ASoC: sunxi: Add bindings for A23/A33/H3 codec's analog path controls
From: Rob Herring @ 2016-11-15 22:22 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Liam Girdwood, Mark Brown, Maxime Ripard, Lee Jones, Mark Rutland,
alsa-devel, linux-arm-kernel, linux-kernel, devicetree,
Mylene Josserand
In-Reply-To: <20161112064648.26779-2-wens@csie.org>
On Sat, Nov 12, 2016 at 02:46:39PM +0800, Chen-Yu Tsai wrote:
> The internal codec on A23/A33/H3 is split into 2 parts. The
> analog path controls are routed through an embedded custom register
> bus accessed through the PRCM block.
>
> The SoCs share a common set of inputs, outputs, and audio paths.
> The following table lists the differences.
>
> ----------------------------------------
> | Feature \ SoC | A23 | A33 | H3 |
> ----------------------------------------
> | Headphone | v | v | |
> ----------------------------------------
> | Line Out | | | v |
> ----------------------------------------
> | Phone In/Out | v | v | |
> ----------------------------------------
>
> Add a binding for this hardware.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> .../devicetree/bindings/sound/sun8i-codec-analog.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 7/9] dt-bindings: input: move ads7846 bindings to touchscreen subdirectory
From: Rob Herring @ 2016-11-15 22:16 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Sebastian Reichel, Dmitry Torokhov, Mark Rutland,
Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
Michael Welling, Mika Penttilä, Javier Martinez Canillas,
Igor Grinberg, Andrew F. Davis, Mark Brown, Jonathan Cameron,
linux-input, devicetree, linux-kernel, linux-omap, letux-kernel,
linux-iio, kernel
In-Reply-To: <3a0b4f67ae4a40b6fea2f61089ef03cc88f29294.1478890925.git.hns@goldelico.com>
On Fri, Nov 11, 2016 at 08:02:04PM +0100, H. Nikolaus Schaller wrote:
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> Documentation/devicetree/bindings/input/{ => touchscreen}/ads7846.txt | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename Documentation/devicetree/bindings/input/{ => touchscreen}/ads7846.txt (100%)
>
> diff --git a/Documentation/devicetree/bindings/input/ads7846.txt b/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/input/ads7846.txt
> rename to Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
> --
> 2.7.3
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Dave Gerlach @ 2016-11-15 22:11 UTC (permalink / raw)
To: Stephen Boyd, Viresh Kumar
Cc: Rob Herring, Mark Brown, Rafael Wysocki, nm, Viresh Kumar,
linaro-kernel, linux-pm, linux-kernel, Vincent Guittot,
devicetree
In-Reply-To: <20161115185645.GA25626@codeaurora.org>
Hi,
On 11/15/2016 12:56 PM, Stephen Boyd wrote:
> On 11/15, Viresh Kumar wrote:
>> On 14-11-16, 18:13, Stephen Boyd wrote:
>>> On 11/14, Rob Herring wrote:
>>>> On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
>>>>> On 10-11-16, 14:51, Stephen Boyd wrote:
>>>>>>
>>>>>> No. The supply names (and also clock names/index) should be left
>>>>>> up to the consumer of the OPP table. We don't want to encode any
>>>>>> sort of details like this between the OPP table and the consumer
>>>>>> of it in DT because then it seriously couples the OPP table to
>>>>>> the consumer device. "The binding" in this case that needs to be
>>>>>> updated is the consumer binding, to indicate that it correlated
>>>>>> foo-supply and bar-supply to index 0 and 1 of the OPP table
>>>>>> voltages.
>>>>>
>>>>> Are you saying that we shall have a property like this then?
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>>>> index ee91cbdd95ee..733946df2fb8 100644
>>>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>>>> @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
>>>>> compatible = "arm,cortex-a7";
>>>>> ...
>>>>>
>>>>> - cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
>>>>> + vcc0-supply = <&cpu_supply0>;
>>>>> + vcc1-supply = <&cpu_supply1>;
>>>>> + vcc2-supply = <&cpu_supply2>;
>>>>> + opp-supply-names = "vcc0", "vcc1", "vcc2";
>>>>
>>>> Uh, no. You already have the names in the *-supply properties. Yes, they
>>>> are a PIA to retrieve compared to a *-names property, but that is the
>>>> nature of this style of binding.
>>
>> Its not just PIA, but impossible AFAICT.
>>
>> There are two important pieces of information we need for multiple
>> regulator support:
>> - Which regulator in the consumer node corresponds to which entry in
>> the OPP table. As Mark mentioned earlier, DT should be able to get
>> us this.
>
> This is also possible from C code though. Or is there some case
> where it isn't possible if we're sharing the same table with two
> devices? I'm lost on when this would ever happen.
>
> It feels like trying to keep the OPP table agnostic of the
> consuming device and the device's binding is more trouble than
> it's worth. Especially considering we have opp-shared and *-name
> now.
I agree with this, I do not like having to pass a list of regulator
names to the opp core that I *hope* the device I am controlling has
provided. The intent seems to be to use the cpufreq-dt driver as is and
not pass any cpu-supply anymore so the cpufreq-dt driver has no
knowledge of what regulators are present (it operates as it would today
on a system with no regulator required). But as is it will move forward
regardless of whether or not we actually intended to provide a multi
regulator set up or platform set_opp helper, and this probably isn't
ideal. I would think cpufreq-dt/opp core should be have knowledge of
what regulators are needed to achieve these opp transitions and make
sure everything is in place before moving ahead.
>
>> - The order in which the supplies need to be programmed. We have all
>> agreed to do this in code instead of inferring it from DT and this
>> patch series already does that.
>
> Agreed. Encoding a sequence into DT doesn't sound very feasible.
> How is this going to be handled though? I don't see any users of
> the code we're reviewing here, so it's hard to grasp how things
> will work. It would be really useful if we had some user of the
> code included in the patch series to get the big picture.
I have sent a patch in reply to the cover letter of this series showing
the driver that I used to test multi regulator on TI am57x platform and
wrote as much detail as I could on how I used what Viresh has provided.
Perhaps that will show how this can be used and help to see what's
missing from the core implementation here.
Previous discussions drove me to pass regulators and necessary values in
the DT but do all sequencing from the driver from fixed code without
inferring anything from the device tree.
Regards,
Dave
>
>>
>> I want to solve the first problem here and I don't see how it can be
>> solved using such entries:
>>
>> cpus {
>> cpu@0 {
>> compatible = "arm,cortex-a7";
>> ...
>>
>> vcc0-supply = <&cpu_supply0>;
>> vcc1-supply = <&cpu_supply1>;
>> vcc2-supply = <&cpu_supply2>;
>> operating-points-v2 = <&cpu0_opp_table>;
>> };
>> };
>>
>> cpu0_opp_table: opp_table0 {
>> compatible = "operating-points-v2";
>> opp-shared;
>>
>> opp@1000000000 {
>> opp-hz = /bits/ 64 <1000000000>;
>> opp-microvolt = <970000>, /* Supply 0 */
>> <960000>, /* Supply 1 */
>> <960000>; /* Supply 2 */
>> };
>> };
>>
>> The code can't figure out which of vcc0, vcc1, vcc2 is added first in
>> the CPU node and so we need to get the order somehow. A separate
>> binding as I mentioned earlier is a probably (ugly) solution.
>>
>>> I think the problem is that Viresh wants the binding to be "self
>>> describing" so that the OPP can be used without a driver knowing
>>> that a supply corresponds to a particular column in the voltage
>>> table.
>>
>> Right, and that's what Mark suggested as well.
>>
>>> I don't understand that though. Can't we set the supply
>>> names from C code somewhere based on the consumer of the OPPs?
>>
>> That's what this patch series is doing right now.
>>
>> So, are you saying that the way this patchset does it is fine with you
>> ?
>
> That's just to handle the ordering of operations? I need to take
> a minute and understand what's changing. You may have spent
> plenty of time developing/updating, but I haven't spent near
> enough time understanding what's going on in these patches to
> give a thorough review.
>
^ permalink raw reply
* Re: specifying order of /dev/mmcblk devices via device-tree?
From: Russell King - ARM Linux @ 2016-11-15 22:10 UTC (permalink / raw)
To: Tim Harvey
Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAJ+vNU2t5f=HQ94_GW-cry7DV3FsR9Vx-ck_E-eNuDk5vd-eaw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Nov 15, 2016 at 01:39:42PM -0800, Tim Harvey wrote:
> On Tue, Nov 15, 2016 at 1:35 PM, Russell King - ARM Linux
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> > On Tue, Nov 15, 2016 at 12:27:53PM -0800, Tim Harvey wrote:
> >> On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
> >> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> >> > So, someone merged a patch which makes mmcblk devices follow the
> >> > host controller numbering.
> >> >
> >> > Now my cubox-i fails to boot correctly because the SD card in the
> >> > _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
> >> >
> >> > USDHC1 is wired to the on-microsom WiFi, and never has anything
> >> > remotely near a SD card or eMMC present. So, this change is
> >> > confusing on these platforms.
> >> >
> >> > Moreover, this is _going_ to break SolidRun distros if people upgrade
> >> > their kernels.
> >> >
> >> > It may be appropriate for eMMC, but it's not appropriate everywhere.
> >> >
> >> > This is a user visible _regression_ in 4.9-rc. Whoever did this,
> >> > please revert whatever change caused this, and next time limit it
> >> > to only eMMC.
> >> >
> >> > Thanks.
> >>
> >> I see the same thing on newer kernels, which is why I asked the
> >> question. I didn't expect (or even want honestly) a non mmcblk0 boot
> >> device and was looking for a way to control that via dt. Now I'm
> >> understanding that to avoid this kind of bootloader/kernel dependence
> >> issue I should be using UUID's to identify the boot device.
> >>
> >> >From my testing it looks like the change your looking for occurred
> >> some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
> >> regression specifically.
> >
> > That depends how you look at it. Yes, there's a change in 4.5 to 4.6
> > which ties the block device number to the host device index, but that's
> > really only part of the story here.
> >
> > 4.8 definitely identifies the SD card in iMX6 usdhc2 as "mmcblk0".
> > 4.9-rc identifies the SD card as "mmcblk1". This makes it a 4.9 change
> > of behaviour - there can be no argument about that.
> >
> > Now, digging further into this today, it appears that:
> >
> > v4.8: usdhc2 was probed first, and is given mmc0.
> > usdhc1 is probed second, and is given mmc1.
> >
> > v4.9-rc: usdhc1 is probed first, and is given mmc0.
> > usdhc2 is probed second, and is given mmc1.
> >
> > I haven't yet been able to figure out why there's been this change
> > of probe order. There's no change that I can see in the iMX6 DT
> > files that would account for this.
> >
>
> I bisected it and the commit your looking for is
> 9aaf3437aa72ed5370bf32c99580a3fa2c330e3d
No it's not.
Let me try and put it plainer:
* Commit 9aaf3437aa72ed5370bf32c99580a3fa2c330e3d ties the mmc block
device number (mmcblkN) to the mmc host interface number (mmcN).
This change happened between 4.5 and 4.6.
* The change I'm seeing happened between 4.8 and 4.9-rc. I'm not
seeing a change of behaviour between 4.5 and 4.6.
* The change I'm seeing changes the order of the physical device
associated with the hosts named mmc0 and mmc1 in the kernel.
* Because physical devices associated with the mmc0 and mmc1 hosts
swap over, the mmcblkN number changes due to the commit you point
out.
* So, the change that I'm seeing between 4.8 and 4.9-rc is not caused
by commit 9aaf3437aa72ed5370bf32c99580a3fa2c330e3d, but by something
else changing the order in which the two usdhc physical hardware
blocks get probed.
Does this make it clearer?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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] dt-binding: remoteproc: Introduce ADSP loader binding
From: Rob Herring @ 2016-11-15 22:08 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ohad Ben-Cohen, Mark Rutland,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Sarangdhar Joshi
In-Reply-To: <1478757009-11522-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Wed, Nov 09, 2016 at 09:50:09PM -0800, Bjorn Andersson wrote:
> This document defines the binding for a component that loads firmware
> and control the life cycle of the Qualcomm ADSP Hexagon core.
>
> Cc: Sarangdhar Joshi <spjoshi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
> Changes since v2:
> - Added the required "xo" clock, from Sarangdhar
> - Added smd-edge node
> - Corrected example
>
> Changes since v1:
> - Added platform names to compatible
>
> .../devicetree/bindings/remoteproc/qcom,adsp.txt | 98 ++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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
* Device Tree track at Linux Plumbers 2016 -- notes, slides
From: Frank Rowand @ 2016-11-15 21:39 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Notes and slides from the Device Tree track at the Linux Plumbers 2016
conference are available at:
http://elinux.org/Device_tree_future#Linux_Plumbers_2016_Device_Tree_Track
--
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: specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-11-15 21:39 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161115213510.GY1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>
On Tue, Nov 15, 2016 at 1:35 PM, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> On Tue, Nov 15, 2016 at 12:27:53PM -0800, Tim Harvey wrote:
>> On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
>> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
>> > So, someone merged a patch which makes mmcblk devices follow the
>> > host controller numbering.
>> >
>> > Now my cubox-i fails to boot correctly because the SD card in the
>> > _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
>> >
>> > USDHC1 is wired to the on-microsom WiFi, and never has anything
>> > remotely near a SD card or eMMC present. So, this change is
>> > confusing on these platforms.
>> >
>> > Moreover, this is _going_ to break SolidRun distros if people upgrade
>> > their kernels.
>> >
>> > It may be appropriate for eMMC, but it's not appropriate everywhere.
>> >
>> > This is a user visible _regression_ in 4.9-rc. Whoever did this,
>> > please revert whatever change caused this, and next time limit it
>> > to only eMMC.
>> >
>> > Thanks.
>>
>> I see the same thing on newer kernels, which is why I asked the
>> question. I didn't expect (or even want honestly) a non mmcblk0 boot
>> device and was looking for a way to control that via dt. Now I'm
>> understanding that to avoid this kind of bootloader/kernel dependence
>> issue I should be using UUID's to identify the boot device.
>>
>> >From my testing it looks like the change your looking for occurred
>> some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
>> regression specifically.
>
> That depends how you look at it. Yes, there's a change in 4.5 to 4.6
> which ties the block device number to the host device index, but that's
> really only part of the story here.
>
> 4.8 definitely identifies the SD card in iMX6 usdhc2 as "mmcblk0".
> 4.9-rc identifies the SD card as "mmcblk1". This makes it a 4.9 change
> of behaviour - there can be no argument about that.
>
> Now, digging further into this today, it appears that:
>
> v4.8: usdhc2 was probed first, and is given mmc0.
> usdhc1 is probed second, and is given mmc1.
>
> v4.9-rc: usdhc1 is probed first, and is given mmc0.
> usdhc2 is probed second, and is given mmc1.
>
> I haven't yet been able to figure out why there's been this change
> of probe order. There's no change that I can see in the iMX6 DT
> files that would account for this.
>
I bisected it and the commit your looking for is
9aaf3437aa72ed5370bf32c99580a3fa2c330e3d
Tim
--
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: specifying order of /dev/mmcblk devices via device-tree?
From: Russell King - ARM Linux @ 2016-11-15 21:35 UTC (permalink / raw)
To: Tim Harvey
Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAJ+vNU38XQBHt709S_z6Quvce6wcqLzRHFuN_bVZ_0eARa3=fw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Nov 15, 2016 at 12:27:53PM -0800, Tim Harvey wrote:
> On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> > So, someone merged a patch which makes mmcblk devices follow the
> > host controller numbering.
> >
> > Now my cubox-i fails to boot correctly because the SD card in the
> > _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
> >
> > USDHC1 is wired to the on-microsom WiFi, and never has anything
> > remotely near a SD card or eMMC present. So, this change is
> > confusing on these platforms.
> >
> > Moreover, this is _going_ to break SolidRun distros if people upgrade
> > their kernels.
> >
> > It may be appropriate for eMMC, but it's not appropriate everywhere.
> >
> > This is a user visible _regression_ in 4.9-rc. Whoever did this,
> > please revert whatever change caused this, and next time limit it
> > to only eMMC.
> >
> > Thanks.
>
> I see the same thing on newer kernels, which is why I asked the
> question. I didn't expect (or even want honestly) a non mmcblk0 boot
> device and was looking for a way to control that via dt. Now I'm
> understanding that to avoid this kind of bootloader/kernel dependence
> issue I should be using UUID's to identify the boot device.
>
> >From my testing it looks like the change your looking for occurred
> some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
> regression specifically.
That depends how you look at it. Yes, there's a change in 4.5 to 4.6
which ties the block device number to the host device index, but that's
really only part of the story here.
4.8 definitely identifies the SD card in iMX6 usdhc2 as "mmcblk0".
4.9-rc identifies the SD card as "mmcblk1". This makes it a 4.9 change
of behaviour - there can be no argument about that.
Now, digging further into this today, it appears that:
v4.8: usdhc2 was probed first, and is given mmc0.
usdhc1 is probed second, and is given mmc1.
v4.9-rc: usdhc1 is probed first, and is given mmc0.
usdhc2 is probed second, and is given mmc1.
I haven't yet been able to figure out why there's been this change
of probe order. There's no change that I can see in the iMX6 DT
files that would account for this.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 v4 4/4] ARM: dts: da850: Add the usb otg device nodeg
From: Bin Liu @ 2016-11-15 21:19 UTC (permalink / raw)
To: Sekhar Nori
Cc: Alexandre Bailon, khilman-rdvid1DuHRBWk0Htik3J/w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <0c94c5e0-9d2c-fff5-1213-2fb2adb20ab0-l0cyMroinI0@public.gmane.org>
On Tue, Nov 15, 2016 at 04:16:02PM +0530, Sekhar Nori wrote:
> On Thursday 03 November 2016 09:29 PM, Alexandre Bailon wrote:
> > This adds the device tree node for the usb otg
> > controller present in the da850 family of SoC's.
> > This also enables the otg usb controller for the lcdk board.
> >
> > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > ---
> > arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
> > arch/arm/boot/dts/da850.dtsi | 15 +++++++++++++++
> > 2 files changed, 23 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> > index 7b8ab21..9f5040c 100644
> > --- a/arch/arm/boot/dts/da850-lcdk.dts
> > +++ b/arch/arm/boot/dts/da850-lcdk.dts
> > @@ -158,6 +158,14 @@
> > rx-num-evt = <32>;
> > };
> >
> > +&usb_phy {
> > + status = "okay";
> > + };
>
> As mentioned by David already, this node needs to be removed. Please
> rebase this on top of latest linux-davinci/master when ready for merging
> (driver changes accepted).
Dropped this patch due to this comment.
Regards,
-Bin.
>
> > +
> > +&usb0 {
> > + status = "okay";
> > +};
> > +
> > &aemif {
> > pinctrl-names = "default";
> > pinctrl-0 = <&nand_pins>;
> > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > index f79e1b9..322a31a 100644
> > --- a/arch/arm/boot/dts/da850.dtsi
> > +++ b/arch/arm/boot/dts/da850.dtsi
> > @@ -372,6 +372,21 @@
> > >;
> > status = "disabled";
> > };
> > + usb_phy: usb-phy {
> > + compatible = "ti,da830-usb-phy";
> > + #phy-cells = <1>;
> > + status = "disabled";
> > + };
> > + usb0: usb@200000 {
> > + compatible = "ti,da830-musb";
> > + reg = <0x200000 0x10000>;
> > + interrupts = <58>;
> > + interrupt-names = "mc";
> > + dr_mode = "otg";
> > + phys = <&usb_phy 0>;
> > + phy-names = "usb-phy";
> > + status = "disabled";
> > + };
>
> Can you separate out the soc specific changes from board changes? Please
> place the usb0 node above the mdio node. I am trying to get to a rough
> ordering based on reg property.
>
> Thanks,
> Sekhar
>
--
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 v4 0/4] Add DT support for DA8xx
From: Bin Liu @ 2016-11-15 21:12 UTC (permalink / raw)
To: Alexandre Bailon
Cc: khilman-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
nsekhar-l0cyMroinI0
In-Reply-To: <1478188752-22447-1-git-send-email-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On Thu, Nov 03, 2016 at 04:59:08PM +0100, Alexandre Bailon wrote:
> Changes in v2:
> * Remove unrelated changes in patch 3
> * Rename the device node in patch 4
>
> Changes in v3:
> * Fix few mistakes in DT binding sample
> * Only build the device table if DT is enabled
>
> Change in v4:
> * Fix a nit
>
> Alexandre Bailon (1):
> ARM: dts: da850: Add the usb otg device node
>
> Petr Kulhavy (3):
> dt/bindings: Add binding for the DA8xx MUSB driver
> usb: musb: core: added helper function for parsing DT
> usb: musb: da8xx: Add DT support for the DA8xx driver
Applied. Thanks.
-Bin.
>
> .../devicetree/bindings/usb/da8xx-usb.txt | 43 ++++++++++++++++++++
> arch/arm/boot/dts/da850-lcdk.dts | 8 ++++
> arch/arm/boot/dts/da850.dtsi | 15 +++++++
> drivers/usb/musb/da8xx.c | 46 ++++++++++++++++++++++
> drivers/usb/musb/musb_core.c | 19 +++++++++
> drivers/usb/musb/musb_core.h | 6 +++
> 6 files changed, 137 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>
> --
> 2.7.3
>
--
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: [PATCH 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Marek Vasut @ 2016-11-15 20:52 UTC (permalink / raw)
To: Shawn Lin, Rob Herring, David Woodhouse, Brian Norris
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478855766-151673-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 11/11/2016 10:16 AM, Shawn Lin wrote:
> Add rockchip serial flash controller driver
>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
[...]
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 4a682ee..48c5e0e 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -65,6 +65,13 @@ config SPI_HISI_SFC
> help
> This enables support for hisilicon SPI-NOR flash controller.
>
> +config SPI_ROCKCHIP_SFC
Keep this list sorted please.
> + tristate "Rockchip Serial Flash Controller(SFC)"
> + depends on ARCH_ROCKCHIP || COMPILE_TEST
> + depends on HAS_IOMEM && HAS_DMA
> + help
> + This enables support for rockchip serial flash controller.
> +
> config SPI_NXP_SPIFI
> tristate "NXP SPI Flash Interface (SPIFI)"
> depends on OF && (ARCH_LPC18XX || COMPILE_TEST)
[...]
> +/* Interrypt mask */
Interrupt :)
> +#define SFC_IMR 0x4
> +#define SFC_IMR_RX_FULL BIT(0)
> +#define SFC_IMR_RX_UFLOW BIT(1)
> +#define SFC_IMR_TX_OFLOW BIT(2)
> +#define SFC_IMR_TX_EMPTY BIT(3)
> +#define SFC_IMR_TRAN_FINISH BIT(4)
> +#define SFC_IMR_BUS_ERR BIT(5)
> +#define SFC_IMR_NSPI_ERR BIT(6)
> +#define SFC_IMR_DMA BIT(7)
[...]
> +enum rockchip_sfc_iftype {
> + IF_TYPE_STD,
> + IF_TYPE_DUAL,
> + IF_TYPE_QUAD,
> +};
> +
> +struct rockchip_sfc {
> + struct device *dev;
> + struct mutex lock;
> + void __iomem *regbase;
> + struct clk *hclk;
> + struct clk *clk;
> + void *buffer;
> + dma_addr_t dma_buffer;
The naming (buffer) could use some improvement or comment for clarification.
> + struct completion cp;
> + struct spi_nor *nor[SFC_MAX_CHIP_NUM];
Should be MAX_CHIPSELECT_NUM , for clarity.
> + u32 num_chip;
u8
> + bool use_dma;
> + bool negative_edge;
Negative edge ... of what ?
> +};
> +
> +struct rockchip_sfc_priv {
> + u32 cs;
Doesn't this board support only 4 CS ? Use u8 :-)
> + u32 clk_rate;
> + struct rockchip_sfc *sfc;
> +};
> +
> +static int get_if_type(enum read_mode flash_read)
> +{
> + enum rockchip_sfc_iftype if_type;
> +
> + switch (flash_read) {
> + case SPI_NOR_DUAL:
> + if_type = IF_TYPE_DUAL;
> + break;
> + case SPI_NOR_QUAD:
> + if_type = IF_TYPE_QUAD;
> + break;
> + case SPI_NOR_NORMAL:
> + case SPI_NOR_FAST:
> + default:
Should the default case really fall back to 1-bit mode or should it
rather report error ?
> + if_type = IF_TYPE_STD;
> + break;
> + }
> +
> + return if_type;
> +}
> +
> +static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
> +{
> + unsigned long timeout = jiffies + HZ;
> + int err = -ETIMEDOUT;
> + u32 status;
> +
> + writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
> +
> + while (time_before(jiffies, timeout)) {
Would readl_poll_*() from include/linux/iopoll.h help here ?
> + status = readl_relaxed(sfc->regbase + SFC_RCVR);
> + if (!(status & SFC_RCVR_RESET)) {
> + err = 0;
> + break;
> + }
> + msleep(20);
> + }
> +
> + if (err)
> + dev_err(sfc->dev, "SFC reset never finished\n");
Should the writel() below be executed if an error happened ?
> + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> + sfc->regbase + SFC_ICLR);
> + return err;
> +}
> +
> +static int rockchip_sfc_init(struct rockchip_sfc *sfc)
> +{
> + int err;
> +
> + err = rockchip_sfc_reset(sfc);
> + if (err)
> + return err;
> +
> + /* Mask all eight interrupts */
> + writel_relaxed(0xff, sfc->regbase + SFC_IMR);
> + /* Phase configure */
What phase ? Please clarify the comment. Also, don't you have to
configure the register if sfc->negative_edge == 0 too ?
> + if (sfc->negative_edge)
> + writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE <<
> + SFC_CTRL_PHASE_SEL_SHIFT,
> + sfc->regbase + SFC_CTRL);
> + return 0;
> +}
> +
> +static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + int ret;
> +
> + mutex_lock(&sfc->lock);
> +
> + ret = clk_set_rate(sfc->clk, priv->clk_rate);
> + if (ret)
> + goto out;
> +
> + ret = clk_prepare_enable(sfc->clk);
> + if (ret)
> + goto out;
> +
> + return 0;
> +
> +out:
> + mutex_unlock(&sfc->lock);
> + return ret;
> +}
> +
> +static void rockchip_sfc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> +
> + clk_disable_unprepare(sfc->clk);
> + mutex_unlock(&sfc->lock);
> +}
> +
> +static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc)
> +{
> + unsigned long timeout = jiffies + 2 * HZ;
> + int err = -ETIMEDOUT;
> + u32 status;
> +
> + /*
> + * Note: tx and rx share the same fifo, so the rx's water level
> + * is the same as rx's, which means this function could be reused
> + * for checking the read operations as well.
> + */
> + while (time_before(jiffies, timeout)) {
readl_poll_*() ?
> + status = readl_relaxed(sfc->regbase + SFC_FSR);
> + if (((status >> SFC_FSR_TX_EMPTY_SHIFT) &
> + SFC_FSR_TX_EMPTY_MASK) == SFC_FSR_TX_IS_EMPTY) {
> + err = 0;
> + break;
> + }
> + msleep(20);
> + }
> +
> + if (err)
> + dev_err(sfc->dev, "SFC tx never empty\n");
> +
> + return err;
> +}
> +
> +static int rockchip_sfc_op_reg(struct spi_nor *nor,
> + u8 opcode, int len, u8 optype)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> +
> + if (((readl_relaxed(sfc->regbase + SFC_FSR) >> SFC_FSR_TX_EMPTY_SHIFT) &
> + SFC_FSR_TX_EMPTY_MASK) != SFC_FSR_TX_IS_EMPTY ||
> + ((readl_relaxed(sfc->regbase + SFC_FSR) >>
> + SFC_FSR_RX_EMPTY_SHIFT) &
> + SFC_FSR_RX_EMPTY_MASK) != SFC_FSR_RX_IS_EMPTY ||
> + (readl_relaxed(sfc->regbase + SFC_SR) == SFC_SR_IS_BUSY))
> + rockchip_sfc_reset(sfc);
This is chaos, please fix this condition so it's actually readable. You
can ie. read the FSR into a variable, do your shifting/anding magic and
then do if (var1 || var2 || var3) {} .
> + reg = (opcode & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT;
> + reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> + reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> + reg |= (optype & SFC_CMD_DIR_MASK) << SFC_CMD_DIR_SHIFT;
> +
> + writel_relaxed(reg, sfc->regbase + SFC_CMD);
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + int ret;
> + u32 tmp;
> + u32 i;
> +
> + ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD);
> + if (ret)
> + return ret;
> +
> + while (len > 0) {
> + tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> + for (i = 0; i < len; i++)
> + *buf++ = (u8)((tmp >> (i * 8)) & 0xff);
Won't this fail for len > 4 ?
Also, you can use ioread32_rep() here, but (!) that won't work for
unaligned reads, which I dunno if they can happen here, but please do
double-check.
> + len = len - 4;
> + }
> +
> + return 0;
> +}
> +
> +static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 words, i;
> +
> + /* Align bytes to words */
> + words = (len + 3) >> 2;
> +
> + for (i = 0; i < words; i++)
> + writel_relaxed(*(buf + 4 * i), sfc->regbase + SFC_DATA);
See above about the ioread32_rep()/iowrite32_rep(), but careful about
unaligned (len % 4 != 0) case.
> + return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR);
> +}
> +
> +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to,
> + dma_addr_t dma_buf, size_t len, u8 op_type)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> + u8 if_type = 0;
> +
> + init_completion(&sfc->cp);
> +
> + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> + sfc->regbase + SFC_ICLR);
> +
> + /* Enable transfer complete interrupt */
> + reg = readl_relaxed(sfc->regbase + SFC_IMR);
> + reg &= ~SFC_IMR_TRAN_FINISH;
> + writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
> + ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
> + SFC_CMD_IDX_SHIFT);
> + else
> + reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
> + ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
> + SFC_CMD_IDX_SHIFT);
reg = nor->read_opcode & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT;
reg |= op_type << SFC_CMD_DIR_SHIFT;
> + reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
> + : SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;
Why don't you just define those SFC_CMD_ADDR_24BITS and co. with the
shift in those bitfields already ? Then you wouldn't have to riddle this
driver with FOO << BAR, but you'd only have FOO all over the place.
> + if_type = get_if_type(nor->flash_read);
> + writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
> + if_type << SFC_CTRL_ADDR_BITS_SHIFT |
> + if_type << SFC_CTRL_CMD_BITS_SHIFT |
Parenthesis missing around the statements ,
(if_type << FOO) | (... << bar)
> + sfc->negative_edge ?
> + SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
> + SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
> + sfc->regbase + SFC_CTRL);
> +
> + reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> + reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +
> + if (op_type == SFC_CMD_DIR_RD)
> + reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
> + SFC_CMD_DUMMY_SHIFT;
Just define SFC_CMD_DUMMY(x) \
(((x) & SFC_CMD_DUMMY_MASK) << SFC_CMD_DUMMY_SHIFT)
And then use it ... reg |= SFC_CMD_DUMMY(nor->read_dummy);
> + /* Should minus one as 0x0 means 1 bit flash address */
> + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
> + writel_relaxed(reg, sfc->regbase + SFC_CMD);
> + writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
> + writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR);
I hope the DMA buffer management is implemented correctly and you're not
running into any weird cache issues.
> + /* Start dma */
> + writel_relaxed(0x1, sfc->regbase + SFC_DMA_TRIGGER);
> +
> + /* Wait for the interrupt. */
> + if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
> + dev_err(sfc->dev, "DMA wait for transfer finish timeout.");
> + return -ETIMEDOUT;
> + }
> +
> + /* Disable transfer finish interrupt */
> + reg = readl_relaxed(sfc->regbase + SFC_IMR);
> + reg |= SFC_IMR_TRAN_FINISH;
> + writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_write(struct rockchip_sfc *sfc, u_char *buf,
> + size_t len)
> +{
> + u32 words, tx_wl, count, i;
> + unsigned long timeout;
> + int ret = 0;
> + u32 *tbuf = (u32 *)buf;
> +
> + /* Align bytes to words */
> + words = (len + 3) >> 2;
> +
> + while (words) {
See iowrite32_rep() above, but I suspect you'll run into problems with
$len which is not multiple of 4 .
> + tx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> + SFC_FSR_TX_WATER_LVL_SHIFT) &
> + SFC_FSR_TX_WATER_LVL_MASK;
> +
> + if (tx_wl > 0) {
> + count = min_t(u32, words, tx_wl);
> + for (i = 0; i < count; i++) {
> + writel_relaxed(*tbuf++,
> + sfc->regbase + SFC_DATA);
> + words--;
> + }
> +
> + if (words == 0)
> + break;
> + timeout = 0;
> + } else {
> + mdelay(1);
> + if (timeout++ > SFC_MAX_IDLE_RETRY) {
> + ret = -ETIMEDOUT;
> + break;
> + }
> + }
> + }
> +
> + if (ret)
> + return ret;
> + else
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
> + size_t len)
> +{
> + u32 words, rx_wl, count, i;
> + unsigned long timeout;
> + int ret = 0;
> + u32 tmp;
> + u32 *tbuf = (u32 *)buf;
> + u_char *tbuf2;
> +
> + words = len >> 2;
> + /* Get the remained bytes */
> + len = len & 0x3;
See above.
> + while (words) {
> + rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> + SFC_FSR_RX_WATER_LVL_SHIFT) &
> + SFC_FSR_RX_WATER_LVL_MASK;
> +
> + if (rx_wl > 0) {
> + count = min_t(u32, words, rx_wl);
> + for (i = 0; i < count; i++) {
> + *tbuf++ = readl_relaxed(sfc->regbase +
> + SFC_DATA);
> + words--;
> + }
> +
> + if (words == 0)
> + break;
> + timeout = 0;
> + } else {
> + mdelay(1);
> + if (timeout++ > SFC_MAX_IDLE_RETRY) {
> + ret = -ETIMEDOUT;
> + break;
> + }
> + }
> + }
> +
> + if (ret)
> + return ret;
> +
> + /* Read the remained bytes */
> + timeout = 0;
> + tbuf2 = (u_char *)tbuf;
> + while (len) {
> + rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> + SFC_FSR_RX_WATER_LVL_SHIFT) &
> + SFC_FSR_RX_WATER_LVL_MASK;
> + if (rx_wl > 0) {
> + tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> + for (i = 0; i < len; i++)
> + tbuf2[i] = (u8)((tmp >> (i * 8)) & 0xff);
> + goto done;
> + } else {
> + mdelay(1);
> + if (timeout++ > SFC_MAX_IDLE_RETRY) {
> + ret = -ETIMEDOUT;
> + break;
> + }
> + }
> + }
> +done:
> + if (ret)
> + return ret;
> + else
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to,
> + size_t len, u_char *buf, u8 op_type)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> + u8 if_type = 0;
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
> + ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
> + SFC_CMD_IDX_SHIFT);
> + else
> + reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
> + ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
> + SFC_CMD_IDX_SHIFT);
See above regarding this condition. I think you can factor out this
common code too. Also nuke the bitshifts , see my comments on
rockchip_sfc_dma_transfer .
> + reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
> + : SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;
> +
> + if_type = get_if_type(nor->flash_read);
> + writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
> + if_type << SFC_CTRL_ADDR_BITS_SHIFT |
> + if_type << SFC_CTRL_CMD_BITS_SHIFT |
> + sfc->negative_edge ?
> + SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
> + SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
> + sfc->regbase + SFC_CTRL);
> +
> + reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> + reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +
> + if (op_type == SFC_CMD_DIR_RD)
> + reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
> + SFC_CMD_DUMMY_SHIFT;
> +
> + /* Should minus one as 0x0 means 1 bit flash address */
> + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
> + writel_relaxed(reg, sfc->regbase + SFC_CMD);
> + writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + return rockchip_sfc_pio_write(sfc, buf, len);
> + else
> + return rockchip_sfc_pio_read(sfc, buf, len);
> +}
> +
> +static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from, size_t len,
> + u_char *read_buf)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + size_t offset;
> + int ret;
> + dma_addr_t dma_addr = 0;
> +
> + if (!sfc->use_dma)
> + goto no_dma;
> +
> + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> + dma_addr = dma_map_single(NULL, (void *)read_buf,
> + trans, DMA_FROM_DEVICE);
> + if (dma_mapping_error(sfc->dev, dma_addr))
> + dma_addr = 0;
> +
> + /* Fail to map dma, use pre-allocated area instead */
> + ret = rockchip_sfc_dma_transfer(nor, from + offset,
> + dma_addr ? dma_addr :
> + sfc->dma_buffer,
> + trans, SFC_CMD_DIR_RD);
> + if (ret) {
> + dev_warn(nor->dev, "DMA read timeout\n");
> + return ret;
> + }
> + if (!dma_addr)
> + memcpy(read_buf + offset, sfc->buffer, trans);
> + }
> +
> + return len;
> +
> +no_dma:
> + ret = rockchip_sfc_pio_transfer(nor, from, len,
> + read_buf, SFC_CMD_DIR_RD);
> + if (ret) {
> + dev_warn(nor->dev, "PIO read timeout\n");
> + return ret;
> + }
> + return len;
> +}
> +
> +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
> + size_t len, const u_char *write_buf)
> +{
> + struct rockchip_sfc_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + size_t offset;
> + int ret;
> + dma_addr_t dma_addr = 0;
> +
> + if (!sfc->use_dma)
> + goto no_dma;
Seems like there's a lot of similarity between read/write .
> + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> + dma_addr = dma_map_single(NULL, (void *)write_buf,
> + trans, DMA_TO_DEVICE);
> + if (dma_mapping_error(sfc->dev, dma_addr)) {
> + dma_addr = 0;
> + memcpy(sfc->buffer, write_buf + offset, trans);
> + }
> +
> + /* Fail to map dma, use pre-allocated area instead */
> + ret = rockchip_sfc_dma_transfer(nor, to + offset,
> + dma_addr ? dma_addr :
> + sfc->dma_buffer,
> + trans, SFC_CMD_DIR_WR);
> + if (dma_addr)
> + dma_unmap_single(NULL, dma_addr,
> + trans, DMA_TO_DEVICE);
> + if (ret) {
> + dev_warn(nor->dev, "DMA write timeout\n");
> + return ret;
> + }
> + }
> +
> + return len;
> +no_dma:
> + ret = rockchip_sfc_pio_transfer(nor, to, len,
> + (u_char *)write_buf, SFC_CMD_DIR_WR);
> + if (ret) {
> + dev_warn(nor->dev, "PIO write timeout\n");
> + return ret;
> + }
> + return len;
> +}
> +
> +/**
> + * Get spi flash device information and register it as a mtd device.
> + */
> +static int rockchip_sfc_register(struct device_node *np,
> + struct rockchip_sfc *sfc)
> +{
> + struct device *dev = sfc->dev;
> + struct spi_nor *nor;
> + struct rockchip_sfc_priv *priv;
> + struct mtd_info *mtd;
> + int ret;
> +
> + nor = devm_kzalloc(dev, sizeof(*nor), GFP_KERNEL);
> + if (!nor)
> + return -ENOMEM;
You can embed struct spi_nor in struct rockchip_sfc_priv and drop this
allocation . Also it'd be a good idea to rename rockchip_sfc_priv to
something like rockchip_sfc_chip_priv to make it explicit this is a
per-chip private data -- which you can even pre-allocate in rockchi_sfc
structure as a static array of (four) such structures (see cadence qspi
driver for how this is done there).
> + nor->dev = dev;
> + spi_nor_set_flash_node(nor, np);
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32(np, "reg", &priv->cs);
> + if (ret) {
> + dev_err(dev, "No reg property for %s\n",
> + np->full_name);
> + return ret;
> + }
> +
> + ret = of_property_read_u32(np, "spi-max-frequency",
> + &priv->clk_rate);
> + if (ret) {
> + dev_err(dev, "No spi-max-frequency property for %s\n",
> + np->full_name);
> + return ret;
> + }
> +
> + priv->sfc = sfc;
> + nor->priv = priv;
> +
> + nor->prepare = rockchip_sfc_prep;
> + nor->unprepare = rockchip_sfc_unprep;
> + nor->read_reg = rockchip_sfc_read_reg;
> + nor->write_reg = rockchip_sfc_write_reg;
> + nor->read = rockchip_sfc_read;
> + nor->write = rockchip_sfc_write;
> + nor->erase = NULL;
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + if (ret)
> + return ret;
> +
> + mtd = &nor->mtd;
> + mtd->name = np->name;
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret)
> + return ret;
> +
> + sfc->nor[sfc->num_chip] = nor;
> + sfc->num_chip++;
> + return 0;
> +}
> +
> +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc)
> +{
> + int i;
> +
> + for (i = 0; i < sfc->num_chip; i++)
> + mtd_device_unregister(&sfc->nor[i]->mtd);
> +}
> +
> +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc)
> +{
> + struct device *dev = sfc->dev;
> + struct device_node *np;
> + int ret;
> +
> + for_each_available_child_of_node(dev->of_node, np) {
> + ret = rockchip_sfc_register(np, sfc);
> + if (ret)
> + goto fail;
> +
> + if (sfc->num_chip == SFC_MAX_CHIP_NUM) {
> + dev_warn(dev, "Exceeds the max cs limitation\n");
> + break;
> + }
> + }
> +
> + return 0;
> +
> +fail:
> + dev_err(dev, "Failed to register all chip\n");
> + rockchip_sfc_unregister_all(sfc);
See cadence qspi where we only unregister the registered flashes.
Implement it the same way here.
> + return ret;
> +}
> +
> +static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
> +{
> + struct rockchip_sfc *sfc = dev_id;
> + u32 reg;
> +
> + reg = readl_relaxed(sfc->regbase + SFC_RISR);
> + dev_dbg(sfc->dev, "Get irq: 0x%x\n", reg);
> +
> + /* Clear interrupt */
> + writel_relaxed(reg, sfc->regbase + SFC_ICLR);
> +
> + if (reg & SFC_IRQ_TRAN_FINISH)
> + complete(&sfc->cp);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int rockchip_sfc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct rockchip_sfc *sfc;
> + int ret;
> +
> + sfc = devm_kzalloc(dev, sizeof(*sfc), GFP_KERNEL);
> + if (!sfc)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, sfc);
> + sfc->dev = dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sfc->regbase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(sfc->regbase))
> + return PTR_ERR(sfc->regbase);
> +
> + sfc->clk = devm_clk_get(&pdev->dev, "sfc");
> + if (IS_ERR(sfc->clk)) {
> + dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
> + return PTR_ERR(sfc->clk);
> + }
> +
> + sfc->hclk = devm_clk_get(&pdev->dev, "hsfc");
> + if (IS_ERR(sfc->hclk)) {
> + dev_err(&pdev->dev, "Failed to get sfc ahp clk\n");
> + return PTR_ERR(sfc->hclk);
> + }
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_warn(dev, "Unable to set dma mask\n");
> + return ret;
> + }
> +
> + sfc->buffer = dmam_alloc_coherent(dev, SFC_DMA_MAX_LEN,
> + &sfc->dma_buffer, GFP_KERNEL);
> + if (!sfc->buffer)
> + return -ENOMEM;
> +
> + mutex_init(&sfc->lock);
> +
> + ret = clk_prepare_enable(sfc->hclk);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable hclk\n");
> + goto err_hclk;
> + }
> +
> + ret = clk_prepare_enable(sfc->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable clk\n");
> + goto err_clk;
> + }
> +
> + if (of_property_read_bool(sfc->dev->of_node, "rockchip,sfc-no-dma"))
> + sfc->use_dma = false;
> + else
> + sfc->use_dma = true;
sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
"rockchip,sfc-no-dma");
> + if (of_device_is_compatible(sfc->dev->of_node,
> + "rockchip,rk1108-sfc"))
> + sfc->negative_edge = true;
> + else
> + sfc->negative_edge = false;
See above
> + /* Find the irq */
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get the irq\n");
> + goto err_irq;
> + }
> +
> + ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
> + 0, pdev->name, sfc);
> + if (ret) {
> + dev_err(dev, "Failed to request irq\n");
> + goto err_irq;
> + }
> +
> + ret = rockchip_sfc_init(sfc);
> + if (ret)
> + goto err_init;
> +
> + ret = rockchip_sfc_register_all(sfc);
> + if (ret)
> + goto err_init;
> +
> + clk_disable_unprepare(sfc->clk);
> + return 0;
> +
> +err_irq:
> +err_init:
Drop the err_irq: label unless you plan to handle the error (which you
should).
> + clk_disable_unprepare(sfc->clk);
> +err_clk:
> + clk_disable_unprepare(sfc->hclk);
> +err_hclk:
> + mutex_destroy(&sfc->lock);
> + return ret;
> +}
> +
> +static int rockchip_sfc_remove(struct platform_device *pdev)
> +{
> + struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
> +
> + rockchip_sfc_unregister_all(sfc);
> + mutex_destroy(&sfc->lock);
> + clk_disable_unprepare(sfc->clk);
> + clk_disable_unprepare(sfc->hclk);
> + return 0;
> +}
> +
> +static const struct of_device_id rockchip_sfc_dt_ids[] = {
> + { .compatible = "rockchip,sfc"},
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
> +
> +static struct platform_driver rockchip_sfc_driver = {
> + .driver = {
> + .name = "rockchip-sfc",
> + .of_match_table = rockchip_sfc_dt_ids,
> + },
> + .probe = rockchip_sfc_probe,
> + .remove = rockchip_sfc_remove,
> +};
> +module_platform_driver(rockchip_sfc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");
MODULE_AUTHOR is missing
--
Best regards,
Marek Vasut
--
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 v2 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers
From: Jyri Sarha @ 2016-11-15 20:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-devicetree, Benoit Cousson, Kevin Hilman, linux-drm,
Tomi Valkeinen, Laurent Pinchart
In-Reply-To: <CAMpxmJVcMa-wfXf1BUXvkZ78v_yWorEka1mkGd3wZ88oyL58Kw@mail.gmail.com>
On 11/15/16 19:36, Bartosz Golaszewski wrote:
> 2016-11-14 17:54 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> Adds drm bride support for attaching drm bridge drivers to tilcdc. The
>> decision whether a video port leads to an external encoder or bridge
>> is made simply based on remote device's compatible string. The code
>> has been tested with BeagleBone-Black with and without BeagleBone
>> DVI-D Cape Rev A3 using ti-tfp410 driver.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>
> Hi Jyri,
>
> thanks a lot for doing this.
>
> One issue I see with this patch is that tilcdc doesn't seem to support
> deferred probe correctly (if modules are built-in). The following
> happens on my setup:
>
> The dump-vga-dac module is loaded first, but the i2c0 is not ready yet
> - probe returns EPROBE_DEFER and it's propagated to tilcdc probe.
>
> [drm] Initialized
> dumb-vga-dac vga_bridge: Couldn't retrieve i2c bus
>
> Then the i2c bus is initialized and dump-vga-dac probe succeeds, but
> the second probe of tilcdc gives me:
>
> [drm:drm_debugfs_init] *ERROR* Cannot create /sys/kernel/debug/dri/64
> [drm:drm_minor_register] *ERROR* DRM: Failed to initialize
> /sys/kernel/debug/dri.
> tilcdc: probe of da8xx_lcdc.0 failed with error -1
>
> I was able to work around this issue by loading modules in correct order.
>
Did you have any conflicts when applying my patch? I have done quite a
few changes lately and especially the initialization sequence and back
off from deferred probe may get broken easily broken if the source base
is not correct. I try to come up with a pull-request candidate branch
soon (hopefully tomorrow) for you to test.
> I then tried testing the patch with a da850-lcdk, but I don't get
> anything on the display (no signal), even though the LCDC seems to
> work fine (modetest and dmesg messages work just like when using the
> tilcdc panel). Also: I see the EDID info is correctly retrieved from
> the display.
>
> Could you take a look at my DT[1] and see if you find it correct?
>
It is hard to follow the dts diff, but if it probes and tilcdc is able
to read EDID modes, there should not be anything more to it.
Cheers,
Jyri
> Best regards,
> Bartosz Golaszewski
>
> [1] http://pastebin.com/dfUX7PyL
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-11-15 20:27 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161114190839.GS1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>
On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> So, someone merged a patch which makes mmcblk devices follow the
> host controller numbering.
>
> Now my cubox-i fails to boot correctly because the SD card in the
> _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
>
> USDHC1 is wired to the on-microsom WiFi, and never has anything
> remotely near a SD card or eMMC present. So, this change is
> confusing on these platforms.
>
> Moreover, this is _going_ to break SolidRun distros if people upgrade
> their kernels.
>
> It may be appropriate for eMMC, but it's not appropriate everywhere.
>
> This is a user visible _regression_ in 4.9-rc. Whoever did this,
> please revert whatever change caused this, and next time limit it
> to only eMMC.
>
> Thanks.
I see the same thing on newer kernels, which is why I asked the
question. I didn't expect (or even want honestly) a non mmcblk0 boot
device and was looking for a way to control that via dt. Now I'm
understanding that to avoid this kind of bootloader/kernel dependence
issue I should be using UUID's to identify the boot device.
>From my testing it looks like the change your looking for occurred
some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
regression specifically.
Regards,
Tim
--
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 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Rob Herring @ 2016-11-15 20:10 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree@vger.kernel.org, Laurent Pinchart, dri-devel,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
linux-media@vger.kernel.org
In-Reply-To: <4350940.43ZWQivOUU@avalon>
On Mon, Nov 14, 2016 at 8:11 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
>> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
>> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>>>> Multiple incompatible data link layers have been used over time to
>> >>>>> transmit image data to LVDS panels. This binding supports display
>> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>>>> specifications.
[...]
>> >>> Furthermore, LVDS data organization is controlled by the combination of
>> >>> both data-mapping and data-mirror. It makes little sense from my point
>> >>> of view to handle one as part of the compatible string and the other one
>> >>> as a separate property.
>> >>>
>> >>>> > +Optional properties:
>> >>>> > +- label: a symbolic name for the panel
>> >>>>
>> >>>> Could be for any panel or display connector.
>> >>>
>> >>> Yes, but I'm not sure to understand how that's relevant :-)
>> >>
>> >> Meaning it should be a common property.
>> >
>> > Sure. So you expect me to reorganize all the panels and connectors DT
>> > bindings in order to get this one merged ? :-)
>>
>> No, because I don't think label is widely defined. Just put it in a
>> common place and reference it. Any other panels can be fixed later.
>> Really, the "simple panel" binding should probably morph into the
>> common binding.
>
> The "label" property is actually defined in the "Devicetree Specification,
Yes, so is reg, interrupts, etc. but we still list when those are used.
> Release 0.1" as recently published on devicetree.org. Where would you like me
> to define it in the bindings ?
Just split things into 2 files. Move everything that's used in other
panel bindings (label, width-mm, ports, etc.) to a panel/common.txt.
Then in lvds-panel.txt, it just refers to common.txt and is just the
LVDS specific things. Bonus points if simple-panel (should just go
away), panel-dpi, panel-dsi-cm are converted.
>> >>>>> +- avdd-supply: reference to the regulator that powers the panel
>> >>>>> analog supply
>> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>>>> digital supply
I would not be against these being common either. It's somewhat better
than simple-panel's "power-supply" property.
>> >>>>
>> >>>> Which one has to be powered on first, what voltage, and with what time
>> >>>> in between? This is why "generic" or "simple" bindings don't work.
>> >>>
>> >>> The above-mentioned specifications also define connectors, pinouts and
>> >>> power supplies, but many LVDS panels compatible with the LVDS physical
>> >>> and data layers use a different connector with small differences in
>> >>> power supplies.
>> >>>
>> >>> I believe the voltage is irrelevant here, it doesn't need to be
>> >>> controlled by the operating system. Power supplies order and timing is
>> >>> relevant, I'll investigate the level of differences between panels. I'm
>> >>> also fine with dropping those properties for now.
>> >>
>> >> Whether you have control of the supplies is dependent on the board.
>> >> Dropping them is just puts us in the simple binding trap. The simple
>> >> bindings start out that way and then people keep adding to them.
>> >
>> > Damn, you can't be fooled easily ;-)
>>
>> I guess you can count all the simple bindings to see how many times I
>> can be fooled. :)
>>
>> > On a more serious note, I'd like to design the bindings in a way that
>> > wouldn't require adding device-specific code in the driver for each panel
>> > model, given that in most cases power supply handling will be generic.
>> > What's your opinion about a generic power supply model that would be used
>> > in the default case, with the option to override it with device-specific
>> > code when needed ?
>>
>> I don't agree. Read Thierry's post on the subject[1].
>
> I'm not sure you understood me correctly (and writing a clarification at
> 4:00am might not help :-)). My point here is that a fair number of panels
> don't care about power sequencing and have no control GPIOs (that's certainly
> the case of the two Mitsubishi panels I use here that have a single power
> supply - good luck trying to convince me that this needs to be sequenced :-) -
> and no enable or reset control pin). This kind of panel should obviously be
> modelled in DT with both a specific and a generic ("panel-lvds") compatible
> string, to ensure that we'll have enough information available on the
> operating system side to control the panel properly. The power supply(ies)
> should be documented in relation to the specific compatible string and not
> mentioned by the generic bindings (whether that should go in one or multiple
> text files is bikeshedding).
Let me rephrase, I don't agree with generic bindings, but I fully
support having a generic driver. So you can have a generic driver
function that can handle your case. Even slightly more complicated can
be handled: turn on all the supplies listed, de-assert any reset
gpios, assert any enable gpios (this is why I push for common naming
of "enable-gpios"). If this is enough for a panel, then the generic
compatible will work. If not, sorry, provide your own power sequencing
code and match on the more specific compatible.
> This being said, on the driver side, I don't see a reason to list the specific
> compatible strings explicitly for those panels when a generic implementation
> matching the generic compatible string can handle them fine. This has no
> impact on the bindings and is an Linux implementation decision.
I think we're in agreement.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v2] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 19:59 UTC (permalink / raw)
To: Felipe Balbi
Cc: Christian Lamparter, linux-kernel, devicetree, linux-usb,
linuxppc-dev, Mark Rutland, Rob Herring, Greg Kroah-Hartman,
Felipe Balbi, John Youn
From: Christian Lamparter <chunkeey@googlemail.com>
This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.
The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")
but without any driver support as the dwc2 driver wasn't available at
that time.
Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting
Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
drivers/usb/dwc2/params.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
- "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
- "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
- "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+ - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
- snps,dwc2: A generic DWC2 USB controller with default parameters.
- reg : Should contain 1 register range (address and length)
- interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..7991c21 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
+ { .compatible = "amcc,dwc-otg", .data = ¶ms_amcc_dwc_otg },
{},
};
MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
--
2.10.0
^ permalink raw reply related
* Re: [PATCH 0/3] usb: dwc2: params revert and rework
From: John Youn @ 2016-11-15 19:37 UTC (permalink / raw)
To: Felipe Balbi, John Youn, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <87polxugfo.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 11/15/2016 3:33 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
>> Hi Felipe,
>>
>> This reverts and fixes a few commits that are queued on your
>> testing/next, removing the previously added DT bindings, and the code
>> that reads them in.
>>
>> The feedback was that IP validation is not reason enough to add these.
>> So we'll leave them out for now.
>
> these are still in testing/next, so I can actually still drop
> them. I'm applying only patch 3/3, please make sure it all looks good on
> testing/next.
>
Ok thanks. There were merge issues when I did that locally so I
figured I'd give you the reverts.
Checked and looks good.
Regards,
John
--
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: [PATCH v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Stephen Boyd @ 2016-11-15 19:27 UTC (permalink / raw)
To: Ritesh Harjani
Cc: adrian.hunter, ulf.hansson, linux-mmc, shawn.lin, andy.gross,
devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
david.griego, stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <3c1a7c72-0ac1-8ed0-87fc-238331f0645b@codeaurora.org>
On 11/15, Ritesh Harjani wrote:
> On 11/15/2016 1:07 AM, Stephen Boyd wrote:
> >On 11/14, Ritesh Harjani wrote:
> >
> >>+}
> >>+
> >>+/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
> >>+static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> >>+{
> >>+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >>+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> >>+ int rc;
> >>+
> >>+ if (!clock) {
> >>+ msm_host->clk_rate = clock;
> >>+ goto out;
> >>+ }
> >>+
> >>+ spin_unlock_irq(&host->lock);
> >>+ if (clock != msm_host->clk_rate) {
> >
> >Why do we need to check here? Can't we call clk_set_rate()
> >Unconditionally?
> Since it may so happen that above layers may call for ->set_clock
> function with same requested clock more than once, hence we cache
> the host->clock here.
> Also, since requested clock (host->clock) can be say 400Mhz but the
> actual pltfm supported clock would be say 384MHz.
clk_set_rate() detects the same rate being set even after it
internally rounds the rate. We're not going to touch the clk
hardware if 400 is requested once but 384 is what's set and then
400 is requested again. Caching the rate here in the driver can
lead to problems too if the driver is out of sync with the clk
hardware state, so it's best to avoid doing anything fancy here
and just let the framework handle duplicates.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 19:08 UTC (permalink / raw)
To: Christian Lamparter, John Youn
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Mark Rutland, Rob Herring, Greg Kroah-Hartman, Felipe Balbi
In-Reply-To: <f9be1906-3d94-e9c6-841a-d7baa2572f75-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On 11/14/2016 3:00 PM, John Youn wrote:
> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>
>>>>>> The device definition was added by:
>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>> available at that time.
>>>>>>
>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>> because of the special ahbcfg configuration. The default
>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> I don't want to add any more of these param structures to the driver
>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>> of using auto-detected defaults and device properties to override
>>>>> them.
>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>
>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>> thinking of making the default INCR.
>>>> Is that actually possible to change the default still? This would
>>>> require to re-evaluate all existing archs/platforms that use
>>>> "snps,dwc2" for INCR16 compatibility.
>>>
>>> INCR, not INCR16, but you're right, so we may not change it even
>>> though though INCR is usually the right choice over INCR4.
>> What about making a device-tree property?
>
> Yes, that's what I meant. I'll send a change for this shortly.
>
>>
>> Recommended properties:
>> - g-ahb-bursts : specifies the ahb bursts length, should be one of
>> "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>> the safer but inefficient "INCR4" is used. The optimal setting is
>> "INCRx".
>>
>> Would this work? If so, I can make a patch over the weekend.
>>> Anyways, with the binding, can't you just set the compatible string to
>>> snps,dwc2?
>>
>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>> a while back about device-tree bindings.
>>
>> They made it very clear to me, that they don't want any generic "catch all
>> compatible" strings:
>>
>> "Bindings should be for hardware (either specific device models, or for
>> classes), and not for Linux drivers. The latter is subject to arbitrary
>> changes while the former is not, as old hardware continues to exist and
>> does not change while drivers get completely reworked." [0]
>>
>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>> and this binding can't be easily changed. Rob Herring explained this in
>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>> to make them work with the changes I made:
>>
>> "You can't remove the old drivers as they are needed to work with
>> old dtbs, so there is no gain.
>>
>> You would need to match on existing compatibles such as
>> moxa,moxart-gpio and provide a match data struct that has all the info
>> you are adding here (e.g. data register offset). Then additionally you
>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>> additional data associated with it. But it has to be new properties,
>> not changing properties. Changing the reg values doesn't work."
>>
>> So, for this to work with the existing canyonlands.dts, I need to have
>> the "amcc,dwc-otg" compatible string.
>
> Ok, if that's the case. But still a bit confused as to what driver was
> working with it before since the binding was not defined for dwc2.
>
>>
>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>> about this case.
>>
>> Regards,
>> Christian
>>
>> [0] <https://patchwork.kernel.org/patch/8976221/>
>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>
>>
>>>>
>>>> From what I can tell based would be:
>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>
>>>>> If that's all you need then a devicetree binding should be enough
>>>>> right?
>>>> Yes. The device is working fine so far.
>>>>
>>>> Regards,
>>>> Christian
>>>>
>>>> ---
>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>
>>>> This patch adds support for the "amcc,usb-otg" device
>>>> which is found in the PowerPC Canyonlands' dts.
>>>>
>>>> The device definition was added by:
>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>> but without any driver support as the dwc2 driver wasn't
>>>> available at that time.
>>>>
>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>> because of the special ahbcfg configuration. The default
>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>> when the USB and SATA is used concurrently.
>>>>
>>>> Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>> Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> v1->v2:
>>>> - moved definitons to params.c
>>>> - removed dma_enable / host_dma parameter
>>>> - added dma_desc_fs_enable parameter
>>>> v2->v3:
>>>> - removed parameters
>>>>
>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>> for ahbcfg.
>>>> ---
>>>> Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>> drivers/usb/dwc2/params.c | 1 +
>>>> 2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 10a2a4b..6ccfe85 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> @@ -12,6 +12,7 @@ Required properties:
>>>> - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>> - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>> - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>> + - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>> - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>> - reg : Should contain 1 register range (address and length)
>>>> - interrupts : Should contain 1 interrupt
>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>> index 64d5c66..9506ab0 100644
>>>> --- a/drivers/usb/dwc2/params.c
>>>> +++ b/drivers/usb/dwc2/params.c
>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>> { .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>> { .compatible = "amlogic,meson8b-usb", .data = ¶ms_amlogic },
>>>> { .compatible = "amlogic,meson-gxbb-usb", .data = ¶ms_amlogic },
>>>> + { .compatible = "amcc,dwc-otg", .data = NULL },
>>>> {},
>>>> };
>>>> MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>
>
> For dwc2 part:
>
> Acked-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>
Hi Felipe,
Can you drop this from your testing/next?
I meant for the 2nd version to be applied, without the params
structure.
I can send you a clean version to apply later today.
Regards,
John
--
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
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