Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible

2018-08-06 Thread Kuninori Morimoto


Hi Eugeniu

Thank you for your reply

> > > Prior to adding M3-N Starter Kit to the list, rename:
> > >  - "renesas,h3ulcb" => "renesas,ulcb"
> > >  - "renesas,m3ulcb" => "renesas,ulcb"
> > 
> > I'm not sure detail, but
> > does it mean, both H3/M3 board can boot/work with
> > "renesas,ulcb" compatible if we had such driver/soc ?
> 
> First, assuming latest vanilla v4.18-rc8 kernel, neither
> "renesas,salvator-x[s]" nor "renesas,(m3|h3)ulcb" compatibles are
> used anywhere outside of DTS and DT bindings documentation:
(snip)
> Since there is no driver using these compatibles, no functional
> breakage is expected by changing the compatible format/name.

Yeah, it is true "so far". I think there is no problem on current kernel.
But, unfortunately we need to keep compatibility for old/new DT
(= actually, I don't like this DT rule. It is 100% "shackles for the legs")
Thus, my big concern is that, in the future,
"if" we added "renesas,ulcb" compatible driver/soc,
both h3/m3 ulcb will use it.
Then, if "h3" can work/boot by using same "m3" settings, it is no problem for me
(= "works but limited" is also OK, of course.
 This means "matched to more generic compatible")

> > My opinion is that if you want to exchange compatible name,
> > related all driver/document should be exchanged in same patch.
(snip)
> For that reason, IMO it might be worth to detach the document updates
> from DTS updates. I have no problems squashing the DTS and doc patches
> into one single commit, but before doing that I would appreciate a
> confirmation from the maintainer. Anyhow, many thanks for your feedback!
(snip)
> It was my impression that the DTS patches are always partitioned
> per-file, to avoid misleading globbing patterns in the commit subjects
> and allow easier DTS commit porting to future SoCs/boards. I will
> gladly follow your suggestion once I get the confirmation from
> maintainer.

Oops, I noticed that Simon was requested from ARM maintainer(?)
to merge/reduce patches
Let's follow Simon's opinion
(This kind of "patch categorize" is based on each ML...)

Best regards
---
Kuninori Morimoto


Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support

2018-08-06 Thread Eugeniu Rosca
Hi Geert,

On Mon, Aug 06, 2018 at 05:15:37PM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
> 
> On Sun, Aug 5, 2018 at 1:14 AM Eugeniu Rosca  wrote:
> > According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
> > interfaces, similar to H3, M3-W and other SoCs from the same family.
> >
> > Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 
> > not found
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 
> > not found
> >
> > CAN support is inspired from below commits:
> >  - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> >  - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> >  - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control 
> > properties")
> >
> > Signed-off-by: Eugeniu Rosca 
> 
> Thanks for your patch!
> 
> Have you actually tested CAN operation, or were you just fixing the
> build failure?
> In case of the latter, please just add minimal placeholders, like were present
> for other device nodes in early versions of r8a77965.dtsi.

Same as replied to Kieran, I will add some skeleton nodes for can0 and
can1, just to avoid DTC errors. It was not my goal to do CAN bring-up.

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

Best regards,
Eugeniu.


Re: [PATCH v2 2/2] mmc: tmio: Fix SCC error detection

2018-08-06 Thread Wolfram Sang
On Mon, Aug 06, 2018 at 04:28:47PM +0200, Niklas Söderlund wrote:
> From: Masaharu Hayakawa 
> 
> SDR104 and HS200 need to check for SCC error. If SCC error is detected,
> retuning is necessary.

Basically good. As of patch 1, we need to clarify HS400 handling. Maybe
this commit message needs a tiny update then :)

> Signed-off-by: Masaharu Hayakawa 
> [Niklas: update commit message]
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> b/drivers/mmc/host/tmio_mmc_core.c
> index 261b4d62d2b1061f..268c4a3b728c775f 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
> *host)
>   if (mrq->cmd->error || (mrq->data && mrq->data->error))
>   tmio_mmc_abort_dma(host);
>  
> - if (host->check_scc_error)
> - host->check_scc_error(host);
> + if (host->check_scc_error && host->check_scc_error(host))
> + mrq->cmd->error = -EILSEQ;
>  
>   /* If SET_BLOCK_COUNT, continue with main command */
>   if (host->mrq && !mrq->cmd->error) {
> -- 
> 2.18.0
> 


signature.asc
Description: PGP signature


Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support

2018-08-06 Thread Eugeniu Rosca
Hi Kieran,

On Mon, Aug 06, 2018 at 12:11:22PM +0100, Kieran Bingham wrote:
> Hi Eugeniu,
> 
> On 05/08/18 00:11, Eugeniu Rosca wrote:
> > According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
> 
> rev 0.55E sounds like rather an old version of this document. Do you
> have access to the later rev1.00 release?

Thanks for this feedback. I was able to find the newer version.

> 
> (Not an issue for this patch itself, I can confirm that revision 1.00
> still confirms M3-N CAN support)
> 
> > interfaces, similar to H3, M3-W and other SoCs from the same family.
> > 
> > Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 
> > not found
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 
> > not found
> 
> Again, this is somewhat referencing the future, as (in patch sequence)
> the file "r8a77965-ulcb-kf.dts" does not yet exist, so does not really
> need to be mentioned here.

Will remove the "r8a77965-ulcb-kf.dtb" line from commit description.

> 
> 
> > CAN support is inspired from below commits:
> >  - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
> >  - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
> >  - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control 
> > properties")
> > 
> > Signed-off-by: Eugeniu Rosca 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32 
> > 
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > index 486aecacb22a..cb8f8573d9ef 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > @@ -656,6 +656,38 @@
> > status = "disabled";
> > };
> >  
> > +   can0: can@e6c3 {
> > +   compatible = "renesas,can-r8a77965",
> > +"renesas,rcar-gen3-can";
> > +   reg = <0 0xe6c3 0 0x1000>;
> > +   interrupts = ;
> > +   clocks = < CPG_MOD 916>,
> > +  < CPG_CORE R8A77965_CLK_CANFD>,
> > +  <_clk>;
> > +   clock-names = "clkp1", "clkp2", "can_clk";
> > +   assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> > +   assigned-clock-rates = <4000>;
> 
> This doesn't look right. Sections 52A.2 has a note stating:
> 
> CANFD? must be set as follows.
> R-Car H3, R-Car M3-W, R-Car M3-N, R-Car V3M, R-Car V3H, R-Car E3: 80 (MHz)
> 
> R-Car D3: 40 (MHz)
> 
> 
> Could you verify / check in case this value should be 80MHz?

Are you sure section "52A. CAN-FD" is the right one for describing the
CAN (non-FD) nodes? For non-FD CAN there is another chapter called
"52. Controller Area Network Interface (CAN interface)". Since the
latter doesn't point out any differences between M3-W and M3-N, I
re-used the M3-W (r8A7796.dtsi) configuration.

FWIW, r8a7795 (H3), r8a7796 (M3) and r8a77995 (D3) all currently
(v4.18-rc8) use the same "assigned-clock-rates" value for can0
and can1 nodes:

$ git grep -E -A 10 "can[01]:" -- arch/arm64/boot/dts/renesas | grep 
assigned-clock-rates
arch/arm64/boot/dts/renesas/r8a7795.dtsi-   assigned-clock-rates = 
<4000>;
arch/arm64/boot/dts/renesas/r8a7795.dtsi-   assigned-clock-rates = 
<4000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi-   assigned-clock-rates = 
<4000>;
arch/arm64/boot/dts/renesas/r8a7796.dtsi-   assigned-clock-rates = 
<4000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi-  assigned-clock-rates = 
<4000>;
arch/arm64/boot/dts/renesas/r8a77995.dtsi-  assigned-clock-rates = 
<4000>;

Anyway, given that this patch only intended to avoid the "make dtbs"
failure and given that any CAN tests are out of scope, I will just leave
a placeholder for can0 and can1 nodes, as suggested by Geert.

> 
> > +   power-domains = < R8A77965_PD_ALWAYS_ON>;
> > +   resets = < 916>;
> > +   status = "disabled";
> > +   };
> > +
> > +   can1: can@e6c38000 {
> > +   compatible = "renesas,can-r8a77965",
> > +"renesas,rcar-gen3-can";
> > +   reg = <0 0xe6c38000 0 0x1000>;
> > +   interrupts = ;
> > +   clocks = < CPG_MOD 915>,
> > +  < CPG_CORE R8A77965_CLK_CANFD>,
> > +  <_clk>;
> > +   clock-names = "clkp1", "clkp2", "can_clk";
> > +   assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> > +   assigned-clock-rates = <4000>;
> 
> Same here of course.
> 
> 
> > +   power-domains = < R8A77965_PD_ALWAYS_ON>;
> > +   resets = < 915>;
> > +   

Re: [PATCH v2 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

2018-08-06 Thread Wolfram Sang

> + !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))

I know this comes from the BSP but I cannot find any documentation why
ignoring SCC errors is different depending on using 4/8 taps.

I'll try to get more internal information...



signature.asc
Description: PGP signature


[PATCH] arm64: dts: renesas: r8a77980: add CSI2/VIN support

2018-08-06 Thread Sergei Shtylyov
Describe the CSI2 and VIN (and their interconnections) in the R8A77980
device tree.

Signed-off-by: Sergei Shtylyov 

---
This patch is against the 'renesas-devel-20180802v2-v4.18-rc7' branch of
Simon Horman's 'renesas.git' repo.

The R8A77980 CSI2/VIN DT binding updates have been posted earlier today...

 arch/arm64/boot/dts/renesas/r8a77980.dtsi |  374 ++
 1 file changed, 374 insertions(+)

Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
===
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -579,6 +579,302 @@
status = "disabled";
};
 
+   vin0: video@e6ef {
+   compatible = "renesas,vin-r8a77980";
+   reg = <0 0xe6ef 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 811>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   resets = < 811>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin0csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+   };
+   };
+
+   vin1: video@e6ef1000 {
+   compatible = "renesas,vin-r8a77980";
+   reg = <0 0xe6ef1000 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 810>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   status = "disabled";
+   resets = < 810>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin1csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+   };
+   };
+
+   vin2: video@e6ef2000 {
+   compatible = "renesas,vin-r8a77980";
+   reg = <0 0xe6ef2000 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 809>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   resets = < 809>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin2csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+   };
+   };
+
+   vin3: video@e6ef3000 {
+   compatible = "renesas,vin-r8a77980";
+   reg = <0 0xe6ef3000 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 808>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   resets = < 808>;
+   status = "disabled";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   reg = <1>;
+
+   vin3csi40: endpoint@2 {
+   reg = <2>;
+   remote-endpoint= <>;
+   };
+   };
+

Re: [PATCH 1/2] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

2018-08-06 Thread Geert Uytterhoeven
Hi Niklas,

On Mon, Aug 6, 2018 at 9:43 PM Niklas Söderlund
 wrote:
> Latest datasheet makes it clear that not all ES revisions of the H3 and
> M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
> unconditionally for these two SoCs. Prepare to handle the quirk based on
> SoC revision instead of compatibility value by using soc_device_match()
> and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.

Thanks for your patch!

> The reason for adding a new quirks struct instead of just a flag is that
> looking ahead it seems more quirks needs to be handled in a SoC revision
> basis.

Do you expect to add a lot? If yes...

> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c

> @@ -569,6 +588,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>
> of_data = of_device_get_match_data(>dev);
>
> +   attr = soc_device_match(sdhi_quirks_match);
> +   if (attr)
> +   quirks = attr->data;

... you may consider just storing a plain struct renesas_sdhi_of_data in
the sdhi_quirks_match[] table.

> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res)
> return -EINVAL;

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support

2018-08-06 Thread Eugeniu Rosca
Hi Kieran,

On Mon, Aug 06, 2018 at 11:56:56AM +0100, Kieran Bingham wrote:
> Hi Eugeniu
> 
> On 05/08/18 00:11, Eugeniu Rosca wrote:
> > After adding CAN support to arch/arm64/boot/dts/renesas/r8a77965.dtsi,
> > checkpatch complained that the new compatible string
> > "renesas,can-r8a77965" is not documented. Fix the warning.
> > 
> 
> Thanks to the correct ordering of your patches, (you have this one
> *before* adding the CAN support to r8a77965) This commit message seems
> to be predicting the future somewhat.
> 
> Perhaps just a simpler commit message would suffice:
> 
> "Document the support for rcar_can on R8A77965 SoC devices."

I like giving the true story behind the patch and the story was that I
was hit by the checkpatch warning, fixed it and re-ordered the commits.
But I will use your version if it sounds better to you.

> 
> > Signed-off-by: Eugeniu Rosca 
> > ---
> >  Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
> > b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > index 94a7f33ac5e9..23264451a5a4 100644
> > --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > @@ -13,6 +13,7 @@ Required properties:
> >   "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
> >   "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
> >   "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
> > + "renesas,can-r8a77965" if CAN controller is a part of R8A77965 
> > SoC.
> >   "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible 
> > device.
> >   "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1
> >   compatible device.
> > @@ -28,9 +29,8 @@ Required properties:
> >  - pinctrl-0: pin control group to be used for this controller.
> >  - pinctrl-names: must be "default".
> >  
> > -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
> > -compatible:
> > -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 
> > clock
> > +Required properties for compatibles [A], [B] and [C]:
> > +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock
> 
> 
> This paragraph could be rewrapped...

Will implement in v2.

> 
> >  and can be used by both CAN and CAN FD controller at the same time. It 
> > needs to
> >  be scaled to maximum frequency if any of these controllers use it. This is 
> > done
> >  using the below properties:
> > @@ -38,6 +38,10 @@ using the below properties:
> >  - assigned-clocks: phandle of clkp2(CANFD) clock.
> >  - assigned-clock-rates: maximum frequency of this clock.
> >  
> > +[A] "renesas,can-r8a7795"
> > +[B] "renesas,can-r8a7796"
> > +[C] "renesas,can-r8a77965"
> > +>  Optional properties:
> >  - renesas,can-clock-select: R-Car CAN Clock Source Select. Valid values 
> > are:
> > <0x0> (default) : Peripheral clock (clkp1)
> > 
> 

Thanks,
Eugeniu.


Re: [PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support

2018-08-06 Thread Sergei Shtylyov
On 08/06/2018 06:21 PM, Sergei Shtylyov wrote:

>> After adding CAN support to arch/arm64/boot/dts/renesas/r8a77965.dtsi,
>> checkpatch complained that the new compatible string
>> "renesas,can-r8a77965" is not documented. Fix the warning.
>>
>> Signed-off-by: Eugeniu Rosca 
>> ---
>>  Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
>> b/Documentation/devicetree/bindings/net/can/rcar_can.txt
>> index 94a7f33ac5e9..23264451a5a4 100644
>> --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> [...]
>> @@ -28,9 +29,8 @@ Required properties:
>>  - pinctrl-0: pin control group to be used for this controller.
>>  - pinctrl-names: must be "default".
>>  
>> -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
>> -compatible:
>> -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 
>> clock
>> +Required properties for compatibles [A], [B] and [C]:
> 
>I'd suggest to avoid the footnotes:
> 
> Required properties for compatibles R8A7795, R8A7796, and R8A77965:

   Oops, the word "compatibles" shouldn't have been there...

>> +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock
>>  and can be used by both CAN and CAN FD controller at the same time. It 
>> needs to
>>  be scaled to maximum frequency if any of these controllers use it. This is 
>> done
>>  using the below properties:
> [...]

MBR, Sergei


Re: [PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support

2018-08-06 Thread Eugeniu Rosca
Hi Sergei,

On Mon, Aug 06, 2018 at 06:21:09PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 08/05/2018 02:11 AM, Eugeniu Rosca wrote:
> 
> > After adding CAN support to arch/arm64/boot/dts/renesas/r8a77965.dtsi,
> > checkpatch complained that the new compatible string
> > "renesas,can-r8a77965" is not documented. Fix the warning.
> > 
> > Signed-off-by: Eugeniu Rosca 
> > ---
> >  Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
> > b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > index 94a7f33ac5e9..23264451a5a4 100644
> > --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> > +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> [...]
> > @@ -28,9 +29,8 @@ Required properties:
> >  - pinctrl-0: pin control group to be used for this controller.
> >  - pinctrl-names: must be "default".
> >  
> > -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
> > -compatible:
> > -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 
> > clock
> > +Required properties for compatibles [A], [B] and [C]:
> 
>I'd suggest to avoid the footnotes:
> 
> Required properties for compatibles R8A7795, R8A7796, and R8A77965:

I like this proposal, since it is the least intrusive and allows future
addition of SoC models with minimum amount of lines changed. Will use it
in v2.

> 
> > +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock
> >  and can be used by both CAN and CAN FD controller at the same time. It 
> > needs to
> >  be scaled to maximum frequency if any of these controllers use it. This is 
> > done
> >  using the below properties:
> [...]
> 
> MBR, Sergei

Thanks,
Eugeniu.


[PATCH 2/2] mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

2018-08-06 Thread Niklas Söderlund
It was though all ES revisions of H3 and M3-W SoCs required the
TMIO_MMC_HAVE_4TAP_HS400 flag. Recent datasheet updates tells us this is
not true, only early ES revisions of the SoC do.

Since quirk matching based on ES revisions is now used to handle the
flag it's possible to align all Gen3 compatibility properties. This will
allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400
mode.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 17 ++--
 2 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 35cc0de6be67a515..d032bd63444d1029 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -82,22 +82,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-   /* DMAC can handle 0x blk count but only 1 segment */
-   .max_blk_count  = 0x,
-   .max_segs   = 1,
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -114,8 +98,8 @@ static const struct renesas_sdhi_of_data 
of_rcar_gen3_compatible = {
 };
 
 static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
{},
 };
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c 
b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 890f192dedbdcc9c..4bb46c489d71f848 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -78,19 +78,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -117,8 +104,8 @@ static const struct of_device_id 
renesas_sdhi_sys_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-r8a7792", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7793", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7794", .data = 
_rcar_gen2_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen1-sdhi", .data = 
_rcar_gen1_compatible, },
{ .compatible = "renesas,rcar-gen2-sdhi", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
-- 
2.18.0



[PATCH 0/2] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

2018-08-06 Thread Niklas Söderlund
Hi,

Recent datasheet updates have made it clear that some quirks are not SoC 
specific but SoC + ES version specific. Currently the quirks are 
selected using compatibility values but whit this new information that 
is not enough.

This small series adds support to select quirks based on SoC + ES 
revision using soc_device_match(). It handles the only quirk upstreamed 
so far, selection of 4-taps for HS400 mode on R-CarH3(ES1.0,ES2.0) and 
R-CarM3W(ES1.0,ES1.1).

As this solution makes the compatible selection method obsolete it is 
removed and all Gen3 compat values now points to the same data 
structure. The specific compats for H3 and M3-W are however kept in the 
driver for backward compatibility.

Based on mmc/next and tested on H3 ES2.0 and M3-N.

Niklas Söderlund (2):
  mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
  mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

 drivers/mmc/host/renesas_sdhi_core.c  | 26 +++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 17 ++--
 3 files changed, 30 insertions(+), 33 deletions(-)

-- 
2.18.0



[PATCH 1/2] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

2018-08-06 Thread Niklas Söderlund
Latest datasheet makes it clear that not all ES revisions of the H3 and
M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
unconditionally for these two SoCs. Prepare to handle the quirk based on
SoC revision instead of compatibility value by using soc_device_match()
and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.

The reason for adding a new quirks struct instead of just a flag is that
looking ahead it seems more quirks needs to be handled in a SoC revision
basis.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_core.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 66e90f233cefcba5..781a31592330f872 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "renesas_sdhi.h"
 #include "tmio_mmc.h"
@@ -48,6 +49,10 @@
 #define SDHI_VER_GEN3_SD   0xcc10
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
+struct renesas_sdhi_quirks {
+   bool hs400_4taps;
+};
+
 static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 {
u32 val;
@@ -555,11 +560,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = {
+   .hs400_4taps = true,
+};
+
+static const struct soc_device_attribute sdhi_quirks_match[]  = {
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_h3_m3w 
},
+   { /* Sentinel. */ },
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
   const struct tmio_mmc_dma_ops *dma_ops)
 {
struct tmio_mmc_data *mmd = pdev->dev.platform_data;
+   const struct renesas_sdhi_quirks *quirks = NULL;
const struct renesas_sdhi_of_data *of_data;
+   const struct soc_device_attribute *attr;
struct tmio_mmc_data *mmc_data;
struct tmio_mmc_dma *dma_priv;
struct tmio_mmc_host *host;
@@ -569,6 +588,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
of_data = of_device_get_match_data(>dev);
 
+   attr = soc_device_match(sdhi_quirks_match);
+   if (attr)
+   quirks = attr->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
@@ -634,6 +657,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_4taps)
+   mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
+
/* For some SoC, we disable internal WP. GPIO may override this */
if (mmc_can_gpio_ro(host->mmc))
mmc_data->capabilities2 &= ~MMC_CAP2_NO_WRITE_PROTECT;
-- 
2.18.0



Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Geert Uytterhoeven
Hi Wolfram,

On Mon, Aug 6, 2018 at 7:04 PM Wolfram Sang  wrote:
> > Please try without that step, or do "echo none > /sys/power/pm_test".
>
> Did this now with a q'n'd hacked gpio driver (see below). Worked like a
> charm. So, the sata_rcar patch under discussion here seems to be fine.
> We only need the GPIO resume on specific boards. This is a seperate
> task. D'accord?

Thanks for testing!

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] ata: sata_rcar: Add r8a77965 support

2018-08-06 Thread Tejun Heo
On Mon, Aug 06, 2018 at 08:05:06PM +0200, Wolfram Sang wrote:
> On Wed, Jul 25, 2018 at 09:16:55PM +0200, Wolfram Sang wrote:
> > Update the binding docs for Renesas R-Car M3-N. No driver changes are
> > needed.
> > 
> > Signed-off-by: Wolfram Sang 
> 
> Tejun, could you pick this also for 4.19?
> 
> Thanks for picking the other patches already!

Sure, applied to libata/for-4.19.

Thanks.

-- 
tejun


Re: [PATCH] ata: sata_rcar: Add r8a77965 support

2018-08-06 Thread Wolfram Sang
On Wed, Jul 25, 2018 at 09:16:55PM +0200, Wolfram Sang wrote:
> Update the binding docs for Renesas R-Car M3-N. No driver changes are
> needed.
> 
> Signed-off-by: Wolfram Sang 

Tejun, could you pick this also for 4.19?

Thanks for picking the other patches already!

> ---
>  Documentation/devicetree/bindings/ata/sata_rcar.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt 
> b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> index e20eac7a3087..4268e17d2411 100644
> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> @@ -8,6 +8,7 @@ Required properties:
> - "renesas,sata-r8a7791" for R-Car M2-W
> - "renesas,sata-r8a7793" for R-Car M2-N
> - "renesas,sata-r8a7795" for R-Car H3
> +   - "renesas,sata-r8a77965" for R-Car M3-N
> - "renesas,rcar-gen2-sata" for a generic R-Car Gen2 
> compatible device
> - "renesas,rcar-gen3-sata" for a generic R-Car Gen3 
> compatible device
> - "renesas,rcar-sata" is deprecated
> -- 
> 2.11.0
> 


signature.asc
Description: PGP signature


Re: [PATCH] ata: sata_rcar: really mask all interrupts on Gen2 and later

2018-08-06 Thread Tejun Heo
On Mon, Aug 06, 2018 at 12:40:05PM +0200, Wolfram Sang wrote:
> Since R-Car Gen2, a new bit has been introduced to the interrupt mask
> register. Update the code to handle it properly as well.
> 
> Signed-off-by: Wolfram Sang 

Applied to libata/for-4.19.

Thanks.

-- 
tejun


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Tejun Heo
On Mon, Aug 06, 2018 at 12:42:00PM +0200, Wolfram Sang wrote:
> From: Masaharu Hayakawa 
> 
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
> 
> Signed-off-by: Masaharu Hayakawa 
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang 

Applied to libata/for-4.19.

Thanks.

-- 
tejun


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Wolfram Sang
Hi Geert,

> Please try without that step, or do "echo none > /sys/power/pm_test".

Did this now with a q'n'd hacked gpio driver (see below). Worked like a
charm. So, the sata_rcar patch under discussion here seems to be fine.
We only need the GPIO resume on specific boards. This is a seperate
task. D'accord?

Thanks,

   Wolfram

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 023a32cfac42..17ab27ca0187 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -914,6 +914,7 @@ static int pca953x_probe(struct i2c_client *client,
dev_warn(>dev, "setup failed, %d\n", ret);
}
 
+   dev_set_drvdata(>dev, chip);
i2c_set_clientdata(client, chip);
return 0;
 
@@ -986,11 +987,27 @@ static const struct of_device_id pca953x_dt_ids[] = {
 
 MODULE_DEVICE_TABLE(of, pca953x_dt_ids);
 
+static int pca953x_resume(struct device *dev)
+{
+   struct pca953x_chip *chip = dev_get_drvdata(dev);
+
+   pca953x_write_regs(chip, chip->regs->output, chip->reg_output);
+
+   pca953x_write_regs(chip, chip->regs->direction,
+   chip->reg_direction);
+   return 0;
+}
+
+static const struct dev_pm_ops pca953x_dev_pm_ops = {
+   .resume = pca953x_resume,
+};
+
 static struct i2c_driver pca953x_driver = {
.driver = {
.name   = "pca953x",
.of_match_table = pca953x_dt_ids,
.acpi_match_table = ACPI_PTR(pca953x_acpi_ids),
+   .pm = _dev_pm_ops,
},
.probe  = pca953x_probe,
.remove = pca953x_remove,



signature.asc
Description: PGP signature


RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

2018-08-06 Thread Chris Brandt
Hi Geert,

On Monday, August 06, 2018 1, Geert Uytterhoeven wrote:
> BTW, it would have been very valuable to know that earlycon didn't work,
> as that
> would have helped in avoiding the earlycon breakage on other parts.

My apologies.

When I had first looked at this months ago, I quickly realized that 
earlycon was not going to work because the RZ/A2 SCIF was not like all the 
other SCIF devices.
So instead I went with a simple 2 line change to DEBUG_LL. After that 
point, I forgot all about earlycon until I saw you mention it this morning.


Chris



Re: [PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support

2018-08-06 Thread Sergei Shtylyov
Hello!

On 08/05/2018 02:11 AM, Eugeniu Rosca wrote:

> After adding CAN support to arch/arm64/boot/dts/renesas/r8a77965.dtsi,
> checkpatch complained that the new compatible string
> "renesas,can-r8a77965" is not documented. Fix the warning.
> 
> Signed-off-by: Eugeniu Rosca 
> ---
>  Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
> b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> index 94a7f33ac5e9..23264451a5a4 100644
> --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
[...]
> @@ -28,9 +29,8 @@ Required properties:
>  - pinctrl-0: pin control group to be used for this controller.
>  - pinctrl-names: must be "default".
>  
> -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
> -compatible:
> -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
> +Required properties for compatibles [A], [B] and [C]:

   I'd suggest to avoid the footnotes:

Required properties for compatibles R8A7795, R8A7796, and R8A77965:

> +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock
>  and can be used by both CAN and CAN FD controller at the same time. It needs 
> to
>  be scaled to maximum frequency if any of these controllers use it. This is 
> done
>  using the below properties:
[...]

MBR, Sergei


Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support

2018-08-06 Thread Geert Uytterhoeven
Hi Eugeniu,

On Sun, Aug 5, 2018 at 1:14 AM Eugeniu Rosca  wrote:
> According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN
> interfaces, similar to H3, M3-W and other SoCs from the same family.
>
> Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 not 
> found
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 not 
> found
>
> CAN support is inspired from below commits:
>  - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
>  - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
>  - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control 
> properties")
>
> Signed-off-by: Eugeniu Rosca 

Thanks for your patch!

Have you actually tested CAN operation, or were you just fixing the
build failure?
In case of the latter, please just add minimal placeholders, like were present
for other device nodes in early versions of r8a77965.dtsi.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Geert Uytterhoeven
Hi Wolfram,

On Mon, Aug 6, 2018 at 5:00 PM Wolfram Sang  wrote:
> > "platform suspend"? Is that s2ram aka a full PSCI system suspend?
>
> It is this:
>
> echo platform > /sys/power/pm_test
> echo mem > /sys/power/state

Thanks!

> Is there a better name for it?

I don't know. It never gets to the actual system suspend step.

> The HDD was also spinning down/up during the cycle...
>
> > I guess not, as it is supposed to fail without PCA9654 suspend/resume 
> > support...
>
> Yeah, I was wondering about that last time, too.
>
> > So more information is needed to convince me ;-)
>
> Here they are :)

With "platform", it doesn't do the full cycle.
Please try without that step, or do "echo none > /sys/power/pm_test".

Thanks!

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible

2018-08-06 Thread Geert Uytterhoeven
On Mon, Aug 6, 2018 at 1:13 PM Geert Uytterhoeven  wrote:
> On Mon, Aug 6, 2018 at 12:38 PM Laurent Pinchart
>  wrote:
> > On Sunday, 5 August 2018 02:11:02 EEST Eugeniu Rosca wrote:
> > > In the context of M3N-ULCB (RTP0RC77965SKBX010SA00) board bring-up, it's
> > > rather pointless to add a new "renesas,m3nulcb" compatible string. Any
> > > SoC-level differences between the two variants of ULCB (M3 and M3-N)
> > > should be successfully covered by making use of existing
> > > "renesas,r8a7796" and "renesas,r8a77965" compatibles.
> > >
> > > Prior to adding M3-N Starter Kit to the list, rename:
> > >  - "renesas,h3ulcb" => "renesas,ulcb"
> > >  - "renesas,m3ulcb" => "renesas,ulcb"
> >
> > This bothers me more than the naming convention in patch 01/14, as this 
> > change
> > would completely hide differences between the H3 and M3-N versions of the
> > ULCB. Compatible strings are listed in a decreasing order of specificity, 
> > and
> > having "renesas,ulcb" as the most-specific compatible string means that the
> > two boards are supposed to be identical, while they are not.
>
> AFAIK the boards are identical (cfr. ), except for the SiP mounted.
> Cfr. e.g. the combined R-Car_StarterKit_Gen3_H3_M3_DEV_Rev.053.pdf
> ("Renesas R-Car H3/M3 Device Manual", incl. schematics).

Sorry, the schematics are in a separate file
R-Car_StarterKit_Gen3_SCH_Rev.110.pdf
with title "R-Car_Gen3 Starterkit", for both the Pro and Premier versions.

But "ULCB" is an unofficial name.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Wolfram Sang

> "platform suspend"? Is that s2ram aka a full PSCI system suspend?

It is this:

echo platform > /sys/power/pm_test
echo mem > /sys/power/state

Is there a better name for it?

The HDD was also spinning down/up during the cycle...

> I guess not, as it is supposed to fail without PCA9654 suspend/resume 
> support...

Yeah, I was wondering about that last time, too.

> So more information is needed to convince me ;-)

Here they are :)



signature.asc
Description: PGP signature


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Sergei Shtylyov
On 08/06/2018 01:42 PM, Wolfram Sang wrote:

> From: Masaharu Hayakawa 
> 
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
> 
> Signed-off-by: Masaharu Hayakawa 
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang 
[...]

Reviewed-by: Sergei Shtylyov 

MBR, Sergei


Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

2018-08-06 Thread Geert Uytterhoeven
Hi Laurent,

On Mon, Aug 6, 2018 at 4:40 PM Laurent Pinchart
 wrote:
> On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> > On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> > >>
> > >> Deriving the proper regshift value from the register block size is
> > >> fragile, as it may have been rounded up.
> > >>
> > >> Furthermore we will need plat_sci_port.regshift again.
> > >
> > > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > > ?
> > >
> > > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > > keep it for them, and only use compat strings for DT platforms, to
> > > avoiding adding back a platform data field.
> >
> > Actually I think the original commit broke SCI on H8/300, as regshift should
> > be zero on that platform.
> > But that was not discovered until recently.
>
> So there are still people booting H3/800 ? :-)

Yes, remember, H8/300 was resurrected with full DT support.
SuperH is the legacy one ;-)

> Could we still keep the mechanism for SH and fix H8/300 with special handling
> somewhere ?

An independent H8/300 fix is "[PATCH v2] serial: sh-sci: byte allocated
register support" (https://www.spinics.net/lists/linux-sh/msg53175.html).

The main issue is: do we bother with regshift or not?
If yes, we have to handle it correctly for both normal serial port handling and
earlycon.
For the latter, we don't have the register block size available.

As of commit 2eaa790989e03900 ("earlycon: Use common framework for
earlycon declarations"), DT and non-DT based earlycon have been merged,
so your original commit may have impacted earlycon on non-DT based
systems, too.  I don't know for sure...

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] ata: sata_rcar: really mask all interrupts on Gen2 and later

2018-08-06 Thread Sergei Shtylyov
Hello!

On 08/06/2018 01:40 PM, Wolfram Sang wrote:

> Since R-Car Gen2, a new bit has been introduced to the interrupt mask
> register. Update the code to handle it properly as well.
> 
> Signed-off-by: Wolfram Sang 
[...]

Reviewed-by: Sergei Shtylyov 

> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
> index 3b8ed10bfb8f..1ad168f76ef3 100644
> --- a/drivers/ata/sata_rcar.c
> +++ b/drivers/ata/sata_rcar.c
[...]

MBR, Sergei


Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

2018-08-06 Thread Geert Uytterhoeven
Hi Laurent,

On Mon, Aug 6, 2018 at 4:37 PM Laurent Pinchart
 wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
>
> Where can that commit be found ?

tty-next

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> > deriving regshift from the register block size (which may be rounded up):
>
> Why should it be rounded up ?

There's no requirement to round it up.  But it's not uncommon to round up
register block sizes to the next power of two, or more.

In hindsight, perhaps we should have adopted the "reg-shift" DT property,
which is used by other serial port drivers, and by of_setup_earlycon().

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

2018-08-06 Thread Laurent Pinchart
On Monday, 6 August 2018 17:37:45 EEST Laurent Pinchart wrote:
> On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
> > Hi all,
> > 
> > This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> 
> Where can that commit be found ?

I found it in -next, sorry for the noise.

> > ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> > support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> 
> > deriving regshift from the register block size (which may be rounded up):
> 
> Why should it be rounded up ?
> 
> >   1. The first patch is an old patch from Sato-san, which I never really
> >  understood.  But it turned out to be a dependency for patch 2.
> >   2. Patch 2 makes sure regshift is initialized when using earlycon,
> >  unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
> >   3. Patch 3 reverts the patch that started deriving regshift from the
> >  register block size, and that removed the plat_sci_port.regshift
> >  field.  Which is a field I needed again in patch 4.
> >   4. Patch 4 removes the remaining regshift derivations on DT platforms.
> >  (5. I didn't bother writing patch 5, which involves adding .regshift
> >  initializations to all SH board files that need it.)
> > 
> > However, I'm not happy with the end result, so please DO NOT apply this!
> > As I spent almost a full day on this, and would still like to know the
> > story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> > post it anyway.
> > 
> > As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car,
> > assuming
> > 
> > no other actions are taken, an alternative solution would be to:
> >   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
> >  SCIx_RZ_SCIFA_REGTYPE"),
> >   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
> >  SCIF address") alternative,
> >   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> > 
> > What do you think?
> > Thanks for your comments!
> > 
> > P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
> > 
> >  are identical?
> > 
> > Geert Uytterhoeven (3):
> >   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
> >   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
> > ports"
> >   [RFC] sh-sci: Derive regshift value from DT compatible value
> > 
> > Yoshinori Sato (1):
> >   [RFC] sh-sci: Use a separate sci_port for earlycon
> >  
> >  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
> >  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
> >  drivers/tty/serial/sh-sci.c   | 68 +--
> >  include/linux/serial_sci.h|  1 +
> >  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart





Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

2018-08-06 Thread Laurent Pinchart
Hi Geert,

On Monday, 6 August 2018 17:34:34 EEST Geert Uytterhoeven wrote:
> On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart wrote:
> > On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> >> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >> 
> >> Deriving the proper regshift value from the register block size is
> >> fragile, as it may have been rounded up.
> >> 
> >> Furthermore we will need plat_sci_port.regshift again.
> > 
> > Won't this break bisection ? Shouldn't you squash patches 3 and 4 together
> > ?
> > 
> > Does this mechanism break anything on non-DT platforms ? If not I'd rather
> > keep it for them, and only use compat strings for DT platforms, to
> > avoiding adding back a platform data field.
> 
> Actually I think the original commit broke SCI on H8/300, as regshift should
> be zero on that platform.
> But that was not discovered until recently.

So there are still people booting H3/800 ? :-)

Could we still keep the mechanism for SH and fix H8/300 with special handling 
somewhere ?

-- 
Regards,

Laurent Pinchart





Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

2018-08-06 Thread Geert Uytterhoeven
Hi Chris,

On Mon, Aug 6, 2018 at 4:18 PM Chris Brandt  wrote:
> On Monday, August 06, 2018 1, linux-sh-ow...@vger.kernel.org wrote:
> > Deriving the proper regshift value from the register block size is
> > fragile (it may have been rounded up in DT, and the mapping granularity
> > is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> > earlycon support (the size is not easily available).
> >
> > On DT systems, derive it from the compatible value instead.
> > This requires adding an entry for RZ/A2 serial ports, which use an
> > atypical regshift value.
>
> I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2
> because earlycon never worked because of RZ/A2's different register locations.

Yeah, sci_probe_regmap() assumed the wrong regtype for your TYPE_SCIF
port. You needed an OF_EARLYCON_DECLARE() line that also filled in
the correct regtype.

BTW, it would have been very valuable to know that earlycon didn't work, as that
would have helped in avoiding the earlycon breakage on other parts.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

2018-08-06 Thread Laurent Pinchart
Hi Geert,

On Monday, 6 August 2018 17:07:51 EEST Geert Uytterhoeven wrote:
>   Hi all,
> 
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a

Where can that commit be found ?

> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):

Why should it be rounded up ?

>   1. The first patch is an old patch from Sato-san, which I never really
>  understood.  But it turned out to be a dependency for patch 2.
>   2. Patch 2 makes sure regshift is initialized when using earlycon,
>  unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
>   3. Patch 3 reverts the patch that started deriving regshift from the
>  register block size, and that removed the plat_sci_port.regshift
>  field.  Which is a field I needed again in patch 4.
>   4. Patch 4 removes the remaining regshift derivations on DT platforms.
>  (5. I didn't bother writing patch 5, which involves adding .regshift
>  initializations to all SH board files that need it.)
> 
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
> 
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
>   1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
>  SCIx_RZ_SCIFA_REGTYPE"),
>   2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
>  SCIF address") alternative,
>   3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
> 
> What do you think?
> Thanks for your comments!
> 
> P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
>  are identical?
> 
> Geert Uytterhoeven (3):
>   [RFC] sh-sci: Take into account regshift to fix earlycon breakage
>   [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
> ports"
>   [RFC] sh-sci: Derive regshift value from DT compatible value
> 
> Yoshinori Sato (1):
>   [RFC] sh-sci: Use a separate sci_port for earlycon
> 
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
>  drivers/tty/serial/sh-sci.c   | 68 +--
>  include/linux/serial_sci.h|  1 +
>  5 files changed, 49 insertions(+), 34 deletions(-)

-- 
Regards,

Laurent Pinchart





Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

2018-08-06 Thread Geert Uytterhoeven
Hi Laurent,

On Mon, Aug 6, 2018 at 4:16 PM Laurent Pinchart
 wrote:
> On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> > This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> >
> > Deriving the proper regshift value from the register block size is
> > fragile, as it may have been rounded up.
> >
> > Furthermore we will need plat_sci_port.regshift again.
>
> Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?
>
> Does this mechanism break anything on non-DT platforms ? If not I'd rather
> keep it for them, and only use compat strings for DT platforms, to avoiding
> adding back a platform data field.

Actually I think the original commit broke SCI on H8/300, as regshift should be
zero on that platform.
But that was not discovered until recently.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH v2 2/2] mmc: tmio: Fix SCC error detection

2018-08-06 Thread Niklas Söderlund
From: Masaharu Hayakawa 

SDR104 and HS200 need to check for SCC error. If SCC error is detected,
retuning is necessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 261b4d62d2b1061f..268c4a3b728c775f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
*host)
if (mrq->cmd->error || (mrq->data && mrq->data->error))
tmio_mmc_abort_dma(host);
 
-   if (host->check_scc_error)
-   host->check_scc_error(host);
+   if (host->check_scc_error && host->check_scc_error(host))
+   mrq->cmd->error = -EILSEQ;
 
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0



[PATCH v2 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

2018-08-06 Thread Niklas Söderlund
Hi,

These patches triggers a retune if a SCC error is detected. They where
ported from the renesas BSP. Masaharu-san did all the real work I just
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where
needed to adapt them to the recent HS400 series picked-up. It's tested
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

 drivers/mmc/host/renesas_sdhi_core.c | 9 +
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.18.0



[PATCH v2 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

2018-08-06 Thread Niklas Söderlund
From: Masaharu Hayakawa 

Checking for SCC error during retuning is unnecessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: fix small style issue]
Signed-off-by: Niklas Söderlund 

---

* Changes since v1
- Use intermediate variable use_4tap to simplify check.
---
 drivers/mmc/host/renesas_sdhi_core.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 777e32b0e410e850..66e90f233cefcba5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -443,6 +443,15 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host 
*host)
 static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
 {
struct renesas_sdhi *priv = host_to_priv(host);
+   bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
+
+   if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))
+   return false;
+
+   if (host->mmc->doing_retune)
+   return false;
 
/* Check SCC error */
if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
-- 
2.18.0



RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

2018-08-06 Thread Chris Brandt
Hi Geert,

On Monday, August 06, 2018 1, linux-sh-ow...@vger.kernel.org wrote:
> Deriving the proper regshift value from the register block size is
> fragile (it may have been rounded up in DT, and the mapping granularity
> is usually PAGE_SIZE anyway), and turned out to be inappropriate for
> earlycon support (the size is not easily available).
> 
> On DT systems, derive it from the compatible value instead.
> This requires adding an entry for RZ/A2 serial ports, which use an
> atypical regshift value.

I had a simple patch to add support for CONFIG_DEBUG_LL for RZ/A2 
because earlycon never worked because of RZ/A2's different register locations.

I'll have to see how things change (better?) with this patch.


Chris


Re: [PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

2018-08-06 Thread Laurent Pinchart
Hi Geert,

Thank you for the patch.

On Monday, 6 August 2018 17:07:54 EEST Geert Uytterhoeven wrote:
> This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.
> 
> Deriving the proper regshift value from the register block size is
> fragile, as it may have been rounded up.
> 
> Furthermore we will need plat_sci_port.regshift again.

Won't this break bisection ? Shouldn't you squash patches 3 and 4 together ?

Does this mechanism break anything on non-DT platforms ? If not I'd rather 
keep it for them, and only use compat strings for DT platforms, to avoiding 
adding back a platform data field.

> Signed-off-by: Geert Uytterhoeven 
> ---
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++
>  drivers/tty/serial/sh-sci.c   |  8 +---
>  include/linux/serial_sci.h|  1 +
>  5 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> b/arch/sh/kernel/cpu/sh3/setup-sh770x.c index
> 59a88611df55ac8f..592cd9ab30c4272f 100644
> --- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> +++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
> @@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
>  static struct plat_sci_port scif0_platform_data = {
>   .type   = PORT_SCI,
>   .ops= _sci_port_ops,
> + .regshift   = 1,
>  };
> 
>  static struct resource scif0_resources[] = {
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7750.c index
> 57d30689204d03b9..d98a55416306baef 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
> @@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
> 
>  static struct plat_sci_port sci_platform_data = {
>   .type   = PORT_SCI,
> + .regshift   = 2,
>  };
> 
>  static struct resource sci_resources[] = {
> - DEFINE_RES_MEM(0xffe0, 0x20),
> + DEFINE_RES_MEM(0xffe0, 0x100),
>   DEFINE_RES_IRQ(evt2irq(0x4e0)),
>  };
> 
> diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> b/arch/sh/kernel/cpu/sh4/setup-sh7760.c index
> e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
> --- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> +++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
> @@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
>  };
> 
>  static struct plat_sci_port scif3_platform_data = {
> - /*
> -  * This is actually a SIM card module serial port, based on an SCI with
> -  * additional registers. The sh-sci driver doesn't support the SIM port
> -  * type, declare it as a SCI. Don't declare the additional registers in
> -  * the memory resource or the driver will compute an incorrect regshift
> -  * value.
> -  */
>   .type   = PORT_SCI,
> + .regshift   = 2,
>  };
> 
>  static struct resource scif3_resources[] = {
> - DEFINE_RES_MEM(0xfe48, 0x10),
> + DEFINE_RES_MEM(0xfe48, 0x100),
>   DEFINE_RES_IRQ(evt2irq(0xc00)),
>   DEFINE_RES_IRQ(evt2irq(0xc20)),
>   DEFINE_RES_IRQ(evt2irq(0xc40)),
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index caf4422d9e2e59e4..955c057dff6e8c78 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device
> *dev,
> 
>   port->type  = p->type;
>   port->flags = UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
> + port->regshift  = p->regshift;
>   port->fifosize  = sci_port->params->fifosize;
> 
> - if (port->type == PORT_SCI) {
> - if (sci_port->reg_size >= 0x20)
> - port->regshift = 2;
> - else
> - port->regshift = 1;
> - }
> -
>   if (regtype == SCIx_SH4_SCIF_REGTYPE)
>   if (sci_port->reg_size >= 0x20)
>   port->regshift = 1;
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index c0e795d95477daea..eebb12fc473f49a2 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -57,6 +57,7 @@ struct plat_sci_port {
>   /*
>* Platform overrides if necessary, defaults otherwise.
>*/
> + unsigned char   regshift;
>   unsigned char   regtype;
> 
>   struct plat_sci_port_ops*ops;

-- 
Regards,

Laurent Pinchart





[PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value

2018-08-06 Thread Geert Uytterhoeven
Deriving the proper regshift value from the register block size is
fragile (it may have been rounded up in DT, and the mapping granularity
is usually PAGE_SIZE anyway), and turned out to be inappropriate for
earlycon support (the size is not easily available).

On DT systems, derive it from the compatible value instead.
This requires adding an entry for RZ/A2 serial ports, which use an
atypical regshift value.

On non-DT systems the regshift value is still derived from the register
block size, as before.

Signed-off-by: Geert Uytterhoeven 
---
  - Sato-san: I assume this fixes SCI on H8/300, too?
Cfr. your patch "serial: sh-sci: byte allocated register support"
(https://www.spinics.net/lists/linux-sh/msg53175.html).

  - Getting rid of the regshift setup for non-DT systems probably means
we'll have to add ".regshift = 1" to each and every SH board file
describing SCIF serial ports :-(

Any other suggestions?
---
 drivers/tty/serial/sh-sci.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 955c057dff6e8c78..c4342e61b8db72c3 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2898,9 +2898,10 @@ static int sci_init_single(struct platform_device *dev,
port->regshift  = p->regshift;
port->fifosize  = sci_port->params->fifosize;
 
-   if (regtype == SCIx_SH4_SCIF_REGTYPE)
+   if (!dev->dev.of_node && regtype == SCIx_SH4_SCIF_REGTYPE) {
if (sci_port->reg_size >= 0x20)
port->regshift = 1;
+   }
 
/*
 * The UART port needs an IRQ value, so we peg this to the RX IRQ
@@ -3100,43 +3101,49 @@ static int sci_remove(struct platform_device *dev)
 }
 
 
-#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype))
+#define SCI_OF_DATA(type, regtype, regshift)   \
+   (void *)((type) << 16 | (regtype) << 8 | (regshift))
 #define SCI_OF_TYPE(data)  ((unsigned long)(data) >> 16)
-#define SCI_OF_REGTYPE(data)   ((unsigned long)(data) & 0x)
+#define SCI_OF_REGTYPE(data)   (((unsigned long)(data) >> 8) & 0xff)
+#define SCI_OF_REGSHIFT(data)  ((unsigned long)(data) & 0xff)
 
 static const struct of_device_id of_sci_match[] = {
/* SoC-specific types */
{
.compatible = "renesas,scif-r7s72100",
-   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH2_SCIF_FIFODATA_REGTYPE,
+   0),
+   }, {
+   .compatible = "renesas,scif-r7s9210",
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 0),
},
/* Family-specific types */
{
.compatible = "renesas,rcar-gen1-scif",
-   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
}, {
.compatible = "renesas,rcar-gen2-scif",
-   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
}, {
.compatible = "renesas,rcar-gen3-scif",
-   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_BRG_REGTYPE, 0),
},
/* Generic types */
{
.compatible = "renesas,scif",
-   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE, 1),
}, {
.compatible = "renesas,scifa",
-   .data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE, 0),
}, {
.compatible = "renesas,scifb",
-   .data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE, 0),
}, {
.compatible = "renesas,hscif",
-   .data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE),
+   .data = SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE, 0),
}, {
.compatible = "renesas,sci",
-   .data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE),
+   .data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, 0),
}, {
/* Terminator */
},
@@ -3179,6 +3186,7 @@ static struct plat_sci_port *sci_parse_dt(struct 
platform_device *pdev,
 
p->type = SCI_OF_TYPE(data);
p->regtype = SCI_OF_REGTYPE(data);
+   p->regshift = SCI_OF_REGSHIFT(data);
 
sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts");
 
-- 
2.17.1



[PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize

2018-08-06 Thread Geert Uytterhoeven
Hi all,

This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
deriving regshift from the register block size (which may be rounded up):
  1. The first patch is an old patch from Sato-san, which I never really
 understood.  But it turned out to be a dependency for patch 2.
  2. Patch 2 makes sure regshift is initialized when using earlycon,
 unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
  3. Patch 3 reverts the patch that started deriving regshift from the
 register block size, and that removed the plat_sci_port.regshift
 field.  Which is a field I needed again in patch 4.
  4. Patch 4 removes the remaining regshift derivations on DT platforms.
 (5. I didn't bother writing patch 5, which involves adding .regshift
 initializations to all SH board files that need it.)

However, I'm not happy with the end result, so please DO NOT apply this!
As I spent almost a full day on this, and would still like to know the
story about "sh-sci: Use a separate sci_port for earlycon", I decided to
post it anyway.

As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
no other actions are taken, an alternative solution would be to:
  1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
 SCIx_RZ_SCIFA_REGTYPE"),
  2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
 SCIF address") alternative,
  3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.

What do you think?
Thanks for your comments!

P.S. Apparently SCIx_SH4_SCIF_REGTYPE and SCIx_SH2_SCIF_FIFODATA_REGTYPE
 are identical?

Geert Uytterhoeven (3):
  [RFC] sh-sci: Take into account regshift to fix earlycon breakage
  [RFC] Revert "serial: sh-sci: Compute the regshift value for SCI
ports"
  [RFC] sh-sci: Derive regshift value from DT compatible value

Yoshinori Sato (1):
  [RFC] sh-sci: Use a separate sci_port for earlycon

 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 +-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 +---
 drivers/tty/serial/sh-sci.c   | 68 +--
 include/linux/serial_sci.h|  1 +
 5 files changed, 49 insertions(+), 34 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


[PATCH/RFC 3/4] Revert "serial: sh-sci: Compute the regshift value for SCI ports"

2018-08-06 Thread Geert Uytterhoeven
This reverts commit dfc80387aefb78161f83732804c6d01c89c24595.

Deriving the proper regshift value from the register block size is
fragile, as it may have been rounded up.

Furthermore we will need plat_sci_port.regshift again.

Signed-off-by: Geert Uytterhoeven 
---
 arch/sh/kernel/cpu/sh3/setup-sh770x.c |  1 +
 arch/sh/kernel/cpu/sh4/setup-sh7750.c |  3 ++-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c | 10 ++
 drivers/tty/serial/sh-sci.c   |  8 +---
 include/linux/serial_sci.h|  1 +
 5 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/sh/kernel/cpu/sh3/setup-sh770x.c 
b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
index 59a88611df55ac8f..592cd9ab30c4272f 100644
--- a/arch/sh/kernel/cpu/sh3/setup-sh770x.c
+++ b/arch/sh/kernel/cpu/sh3/setup-sh770x.c
@@ -111,6 +111,7 @@ static struct platform_device rtc_device = {
 static struct plat_sci_port scif0_platform_data = {
.type   = PORT_SCI,
.ops= _sci_port_ops,
+   .regshift   = 1,
 };
 
 static struct resource scif0_resources[] = {
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7750.c 
b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
index 57d30689204d03b9..d98a55416306baef 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7750.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7750.c
@@ -39,10 +39,11 @@ static struct platform_device rtc_device = {
 
 static struct plat_sci_port sci_platform_data = {
.type   = PORT_SCI,
+   .regshift   = 2,
 };
 
 static struct resource sci_resources[] = {
-   DEFINE_RES_MEM(0xffe0, 0x20),
+   DEFINE_RES_MEM(0xffe0, 0x100),
DEFINE_RES_IRQ(evt2irq(0x4e0)),
 };
 
diff --git a/arch/sh/kernel/cpu/sh4/setup-sh7760.c 
b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
index e51fe1734e1368e8..0c0cdfc69dcc3e33 100644
--- a/arch/sh/kernel/cpu/sh4/setup-sh7760.c
+++ b/arch/sh/kernel/cpu/sh4/setup-sh7760.c
@@ -200,18 +200,12 @@ static struct platform_device scif2_device = {
 };
 
 static struct plat_sci_port scif3_platform_data = {
-   /*
-* This is actually a SIM card module serial port, based on an SCI with
-* additional registers. The sh-sci driver doesn't support the SIM port
-* type, declare it as a SCI. Don't declare the additional registers in
-* the memory resource or the driver will compute an incorrect regshift
-* value.
-*/
.type   = PORT_SCI,
+   .regshift   = 2,
 };
 
 static struct resource scif3_resources[] = {
-   DEFINE_RES_MEM(0xfe48, 0x10),
+   DEFINE_RES_MEM(0xfe48, 0x100),
DEFINE_RES_IRQ(evt2irq(0xc00)),
DEFINE_RES_IRQ(evt2irq(0xc20)),
DEFINE_RES_IRQ(evt2irq(0xc40)),
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index caf4422d9e2e59e4..955c057dff6e8c78 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2895,15 +2895,9 @@ static int sci_init_single(struct platform_device *dev,
 
port->type  = p->type;
port->flags = UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p->flags;
+   port->regshift  = p->regshift;
port->fifosize  = sci_port->params->fifosize;
 
-   if (port->type == PORT_SCI) {
-   if (sci_port->reg_size >= 0x20)
-   port->regshift = 2;
-   else
-   port->regshift = 1;
-   }
-
if (regtype == SCIx_SH4_SCIF_REGTYPE)
if (sci_port->reg_size >= 0x20)
port->regshift = 1;
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index c0e795d95477daea..eebb12fc473f49a2 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -57,6 +57,7 @@ struct plat_sci_port {
/*
 * Platform overrides if necessary, defaults otherwise.
 */
+   unsigned char   regshift;
unsigned char   regtype;
 
struct plat_sci_port_ops*ops;
-- 
2.17.1



[PATCH/RFC 1/4] sh-sci: Use a separate sci_port for earlycon

2018-08-06 Thread Geert Uytterhoeven
From: Yoshinori Sato 

The first sh-sci port and earlycon use the same sci_port.
Hence after the initialization of the first port, earlycon may die.

Fis this by assigning a separate sci_port for earlycon.

Signed-off-by: Yoshinori Sato 
[geert: Rebased, reworded]
Signed-off-by: Geert Uytterhoeven 
---
I still don't fully understand why Sato-san needed this for H8/300, and
we never needed it on Renesas ARM SoCs before.

Also, I don't like the early index.
---
 drivers/tty/serial/sh-sci.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index ac4424bf6b136cc4..cf3195bed0246a3d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -164,6 +164,8 @@ struct sci_port {
 #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
 
 static struct sci_port sci_ports[SCI_NPORTS];
+#define EARLY_INDEX 256
+static struct sci_port earlycon_port;
 static unsigned long sci_ports_in_use;
 static struct uart_driver sci_uart_driver;
 
@@ -2941,12 +2943,16 @@ static void serial_console_putchar(struct uart_port 
*port, int ch)
 static void serial_console_write(struct console *co, const char *s,
 unsigned count)
 {
-   struct sci_port *sci_port = _ports[co->index];
-   struct uart_port *port = _port->port;
unsigned short bits, ctrl, ctrl_temp;
+   struct sci_port *sci_port;
+   struct uart_port *port;
unsigned long flags;
int locked = 1;
 
+   sci_port = (co->index != EARLY_INDEX) ?
+   _ports[co->index] : _port;
+   port = _port->port;
+
 #if defined(SUPPORT_SYSRQ)
if (port->sysrq)
locked = 0;
@@ -3367,12 +3373,13 @@ static int __init early_console_setup(struct 
earlycon_device *device,
device->port.serial_in = sci_serial_in;
device->port.serial_out = sci_serial_out;
device->port.type = type;
-   memcpy(_ports[0].port, >port, sizeof(struct uart_port));
+   device->con->index = EARLY_INDEX;
+   memcpy(_port.port, >port, sizeof(struct uart_port));
port_cfg.type = type;
-   sci_ports[0].cfg = _cfg;
-   sci_ports[0].params = sci_probe_regmap(_cfg);
-   port_cfg.scscr = sci_serial_in(_ports[0].port, SCSCR);
-   sci_serial_out(_ports[0].port, SCSCR,
+   earlycon_port.cfg = _cfg;
+   earlycon_port.params = sci_probe_regmap(_cfg);
+   port_cfg.scscr = sci_serial_in(_port.port, SCSCR);
+   sci_serial_out(_port.port, SCSCR,
   SCSCR_RE | SCSCR_TE | port_cfg.scscr);
 
device->con->write = serial_console_write;
-- 
2.17.1



[PATCH/RFC 2/4] sh-sci: Take into account regshift to fix earlycon breakage

2018-08-06 Thread Geert Uytterhoeven
With the compressed SCIF address range, the SCIF ports on most Renesas
SoCs now use regshift 1.

However, the earlycon setup code always assumes regshift zero, breaking
earlycon on R-Car, RZ/A1, and RZ/G SoCs.

Fix this by initializing regshift to 1 for the generic SCIF case, and
adding a special entry for RZ/A2 SoCs that do need regshift 0.

Signed-off-by: Geert Uytterhoeven 
Fixes: 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed SCIF address")
---
Tested on R-Car Gen2 and Gen3.

This depends on the previous patch.  Else the kernel hangs when
initializing SCIF0 (why??).
---
 drivers/tty/serial/sh-sci.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index cf3195bed0246a3d..caf4422d9e2e59e4 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3390,9 +3390,17 @@ static int __init sci_early_console_setup(struct 
earlycon_device *device,
 {
return early_console_setup(device, PORT_SCI);
 }
+static int __init rza2_early_console_setup(struct earlycon_device *device,
+ const char *opt)
+{
+   /* RZ/A2 uses the default regshift zero */
+   return early_console_setup(device, PORT_SCIF);
+}
 static int __init scif_early_console_setup(struct earlycon_device *device,
  const char *opt)
 {
+   /* Other plain SCIF variants */
+   device->port.regshift = 1;
return early_console_setup(device, PORT_SCIF);
 }
 static int __init scifa_early_console_setup(struct earlycon_device *device,
@@ -3412,6 +3420,7 @@ static int __init hscif_early_console_setup(struct 
earlycon_device *device,
 }
 
 OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
+OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rza2_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
 OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
-- 
2.17.1



Re: [PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible

2018-08-06 Thread Vladimir Zapolskiy
Hi Laurent,

[Adding Rob and DT ML]

On 08/06/2018 01:42 PM, Laurent Pinchart wrote:
> Hi Eugeniu,
> 
> Thank you for the patch.
> 
> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote:
>> Following the recent change in dt-bindings [1], switch from
>> "renesas,h3ulcb" to "renesas,ulcb" compatible string.
>>
>> [1] Documentation/devicetree/bindings/arm/shmobile.txt
>>
>> Signed-off-by: Eugeniu Rosca 
>> ---
>>  arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index
>> 06deb67c36c8..7a5b1dc64090 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> @@ -14,6 +14,6 @@
>>
>>  / {
>>  model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x";
>> -compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
>> +compatible = "shimafuji,kingfisher", "renesas,ulcb",
>>   "renesas,r8a7795";
> 
> This is unrelated to your patch, but due to the reason explained in my review 
> of 02/14, I think "shimafuji,kingfisher" should include the SoC name.
> 
> This brings up the topic of how to describe boards that are made of an SoC 
> "module" board plugged into an expansion "motherboard".
> 

Diving into (probably shatteredly recollected by me) history, a board
'compatible' property appears as a handle to deal with incomplete board
description represented in DTB, so that arch code or drivers can catch on it
and fill some board specific missing parts in runtime, commonly the related
code takes a shape of quirks.

If we accept it, SoC specific drivers would take their interest in SoC model
and revision, and out-of-SoC/platform/board drivers and legacy arch board
code can look at a PCB board model variant. Note that for both separated
but comparably large and important pieces of board/platform knowledge there
is a single compatible property in use, namely the compatible property of
the root node. Also note that both ePAPR and Devicetree Specification describe
'compatible' property of the root node as a special one, as "a list of
platform architectures with which this platform is compatible".

In my opinion a better board representation would be to add a 'soc' device
node as a child of the root node, and the former one has its own fixed
compatible property, but a protocol based on such view has to be agreed
and accepted firstly, and it falls into "forever unrealistic tasks" category.

Today the SoC part of compatible property value is still in active use, and
PCB\SoC part is almost abandoned, so I would propose to diminish the asset
of the latter. Since both board/platform descriptions are alloyed in one
list of values, I'd like to see a better description of acceptable values
of 'compatible' property of the root node.

The common practice is to put SoC specific values into the right tail,
and place (kind of optional) board specific values on the left, let's
continue to follow it, but unrestrict SoC agnostic string names of boards
and further board extensions, if PCB\SoC (or PCB\PCB for extension boards)
are about identical. Anyway those values are mainly unused nowadays, so
nothing is lost but another dimension in board/platform description and
naming is avoided. I do understand that likely most of PCBs are very SoC
centric, and the proposal shared above should not be considered as a rule,
but rather as a reasonable and valid exception.

--
Best wishes,
Vladimir


Re: [PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Geert Uytterhoeven
Hi Wolfram,

On Mon, Aug 6, 2018 at 12:43 PM Wolfram Sang
 wrote:
> From: Masaharu Hayakawa 
>
> According to documentation, setting of PHY registers is unnecessary with
> R-Car Gen3. The registers are not even described. So, don't initialize
> them.
>
> Signed-off-by: Masaharu Hayakawa 
> [wsa: updated commit message]
> Signed-off-by: Wolfram Sang 

Thanks for your patch!

> Tested with a Renesas R-Car M3N Salvator-XS board. I did a platform suspend to
> check if the controller and PHY work properly after resume.

"platform suspend"? Is that s2ram aka a full PSCI system suspend?
I guess not, as it is supposed to fail without PCA9654 suspend/resume support...

So more information is needed to convince me ;-)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible

2018-08-06 Thread Geert Uytterhoeven
Hi Laurent,

On Mon, Aug 6, 2018 at 12:38 PM Laurent Pinchart
 wrote:
> On Sunday, 5 August 2018 02:11:02 EEST Eugeniu Rosca wrote:
> > In the context of M3N-ULCB (RTP0RC77965SKBX010SA00) board bring-up, it's
> > rather pointless to add a new "renesas,m3nulcb" compatible string. Any
> > SoC-level differences between the two variants of ULCB (M3 and M3-N)
> > should be successfully covered by making use of existing
> > "renesas,r8a7796" and "renesas,r8a77965" compatibles.
> >
> > Prior to adding M3-N Starter Kit to the list, rename:
> >  - "renesas,h3ulcb" => "renesas,ulcb"
> >  - "renesas,m3ulcb" => "renesas,ulcb"
>
> This bothers me more than the naming convention in patch 01/14, as this change
> would completely hide differences between the H3 and M3-N versions of the
> ULCB. Compatible strings are listed in a decreasing order of specificity, and
> having "renesas,ulcb" as the most-specific compatible string means that the
> two boards are supposed to be identical, while they are not.

AFAIK the boards are identical (cfr. ), except for the SiP mounted.
Cfr. e.g. the combined R-Car_StarterKit_Gen3_H3_M3_DEV_Rev.053.pdf
("Renesas R-Car H3/M3 Device Manual", incl. schematics).

Hence to me the patch makes sense (modulo out-of-tree dependencies on the
old compatible values).

> > Relevant DTS changes come in separate per-DTS commits.
> >
> > Signed-off-by: Eugeniu Rosca 
> > ---
> >  Documentation/devicetree/bindings/arm/shmobile.txt | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt
> > b/Documentation/devicetree/bindings/arm/shmobile.txt index
> > d8cf740132c6..f391dba10574 100644
> > --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> > @@ -81,7 +81,7 @@ Boards:
> >  compatible = "renesas,gose", "renesas,r8a7793"
> >- H3ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
> >  H3ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
> > -compatible = "renesas,h3ulcb", "renesas,r8a7795"
> > +compatible = "renesas,ulcb", "renesas,r8a7795"
> >- Henninger
> >  compatible = "renesas,henninger", "renesas,r8a7791"
> >- iWave Systems RZ/G1C Single Board Computer (iW-RainboW-G23S)
> > @@ -105,7 +105,7 @@ Boards:
> >- Lager (RTP0RC7790SEB00010S)
> >  compatible = "renesas,lager", "renesas,r8a7790"
> >- M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
> > -compatible = "renesas,m3ulcb", "renesas,r8a7796"
> > +compatible = "renesas,ulcb", "renesas,r8a7796"
> >- Marzen (R0P7779A00010S)
> >  compatible = "renesas,marzen", "renesas,r8a7779"
> >- Porter (M2-LCDP)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


RE: [PATCH v2 2/5] dt-bindings: gpio: rcar: Add gpio-reserved-ranges support

2018-08-06 Thread Biju Das
Hi Linus,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/5] dt-bindings: gpio: rcar: Add gpio-reserved-ranges
> support

> > Update the DT bindings documentation with the optional
> > gpio-reserved-ranges properties.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> (...)
> > +  - gpio-reserved-ranges: Set of tuples to specify the unused GPIOs.This
> > +property indicates the start and size of the GPIOs that can't be used.
> > +
> >  Please refer to gpio.txt in this directory for details of gpio-ranges
> > property  and the common GPIO bindings used by client devices.
>
> You see it yourself if you look at the context below: just refer to the 
> existing
> documentation in gpio.txt as this is a standard bindings.
>
> The absolutely best (IIUC) is:
>
> gpio-ranges: See gpio.txt
> gpio-reserved-ranges: See gpio.txt
OK. Will add this.
> Maybe you could add an extra example using the reserved ranges?
OK.  Will send an example with reserved ranges.

Regards,
Biju



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.


Re: [PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support

2018-08-06 Thread Kieran Bingham
Hi Eugeniu,

On 05/08/18 00:11, Eugeniu Rosca wrote:
> According to R-Car Gen3 HW manual rev0.55E, R-Car M3-N has two CAN

rev 0.55E sounds like rather an old version of this document. Do you
have access to the later rev1.00 release?

(Not an issue for this patch itself, I can confirm that revision 1.00
still confirms M3-N CAN support)

> interfaces, similar to H3, M3-W and other SoCs from the same family.
> 
> Add CAN nodes to avoid below r8a77965-ulcb-kf.dtb build failure:
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:19.1-6 Label or path can0 not 
> found
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:25.1-6 Label or path can1 not 
> found

Again, this is somewhat referencing the future, as (in patch sequence)
the file "r8a77965-ulcb-kf.dts" does not yet exist, so does not really
need to be mentioned here.


> CAN support is inspired from below commits:
>  - v4.7 commit 308b7e4ba62e ("arm64: dts: r8a7795: Add CAN support")
>  - v4.11 commit 909c16252415 ("arm64: dts: r8a7796: Add CAN support")
>  - v4.12 commit bec0948e810f ("arm64: dts: r8a7796: Add reset control 
> properties")
> 
> Signed-off-by: Eugeniu Rosca 
> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 32 
> 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> index 486aecacb22a..cb8f8573d9ef 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -656,6 +656,38 @@
>   status = "disabled";
>   };
>  
> + can0: can@e6c3 {
> + compatible = "renesas,can-r8a77965",
> +  "renesas,rcar-gen3-can";
> + reg = <0 0xe6c3 0 0x1000>;
> + interrupts = ;
> + clocks = < CPG_MOD 916>,
> +< CPG_CORE R8A77965_CLK_CANFD>,
> +<_clk>;
> + clock-names = "clkp1", "clkp2", "can_clk";
> + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> + assigned-clock-rates = <4000>;

This doesn't look right. Sections 52A.2 has a note stating:

CANFD? must be set as follows.
R-Car H3, R-Car M3-W, R-Car M3-N, R-Car V3M, R-Car V3H, R-Car E3: 80 (MHz)

R-Car D3: 40 (MHz)


Could you verify / check in case this value should be 80MHz?

> + power-domains = < R8A77965_PD_ALWAYS_ON>;
> + resets = < 916>;
> + status = "disabled";
> + };
> +
> + can1: can@e6c38000 {
> + compatible = "renesas,can-r8a77965",
> +  "renesas,rcar-gen3-can";
> + reg = <0 0xe6c38000 0 0x1000>;
> + interrupts = ;
> + clocks = < CPG_MOD 915>,
> +< CPG_CORE R8A77965_CLK_CANFD>,
> +<_clk>;
> + clock-names = "clkp1", "clkp2", "can_clk";
> + assigned-clocks = < CPG_CORE R8A77965_CLK_CANFD>;
> + assigned-clock-rates = <4000>;

Same here of course.


> + power-domains = < R8A77965_PD_ALWAYS_ON>;
> + resets = < 915>;
> + status = "disabled";
> + };
> +
>   pwm0: pwm@e6e3 {
>   compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
>   reg = <0 0xe6e3 0 8>;
> 



Re: [PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible

2018-08-06 Thread Laurent Pinchart
Hi Kieran,

On Monday, 6 August 2018 13:49:59 EEST Kieran Bingham wrote:
> On 06/08/18 11:45, Kieran Bingham wrote:
> > On 06/08/18 11:42, Laurent Pinchart wrote:
> >> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote:
> >>> Following the recent change in dt-bindings [1], switch from
> >>> "renesas,h3ulcb" to "renesas,ulcb" compatible string.
> >>> 
> >>> [1] Documentation/devicetree/bindings/arm/shmobile.txt
> >>> 
> >>> Signed-off-by: Eugeniu Rosca 
> >>> ---
> >>> 
> >>>  arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> >>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index
> >>> 06deb67c36c8..7a5b1dc64090 100644
> >>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> >>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> >>> @@ -14,6 +14,6 @@
> >>> 
> >>>  / {
> >>>   model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x";
> >>> 
> >>> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
> >>> + compatible = "shimafuji,kingfisher", "renesas,ulcb",
> >>>"renesas,r8a7795";
> >> 
> >> This is unrelated to your patch, but due to the reason explained in my
> >> review of 02/14, I think "shimafuji,kingfisher" should include the SoC
> >> name.
> >> 
> >> This brings up the topic of how to describe boards that are made of an
> >> SoC "module" board plugged into an expansion "motherboard".
> > 
> > Isn't it the point that the shimafuji board is agnostic to the SoC on
> > the ULCB?
> > 
> > I presume the Kingfisher board is just the expansion board which would
> > be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB?
> 
> In fact possibly the interesting point is that the kingfisher as an
> expansion board is surely an 'overlay', rather than the board itself ?

In a way it's both. From an SoC point of view, the KingFisher board can be 
seen as an expansion board. From a system point of view, it's a KingFisher 
system.

If you built a car around an H3 ULCB, would you consider that to be an H3 with 
a car extension, or a car with an H3 inside ? :-)

> >>>  };

-- 
Regards,

Laurent Pinchart





Re: [PATCH] gpiolib: Fix of_node inconsistency

2018-08-06 Thread Linus Walleij
On Mon, Aug 6, 2018 at 11:53 AM Biju Das  wrote:

> Some platforms are not setting of_node in the driver. On these platforms
> defining gpio-reserved-ranges on device tree leads to kernel crash.
>
> It is due to some parts of the gpio core relying on the driver to set up
> of_node,while other parts do themselves.This inconsistent behaviour leads
> to a crash.
>
> gpiochip_add_data_with_key() calls gpiochip_init_valid_mask() with of_node
> as NULL. of_gpiochip_add() fills "of_node" and calls
> of_gpiochip_init_valid_mask().
>
> The fix is to move the assignment to chip->of_node from of_gpiochip_add()
> to gpiochip_add_data_with_key().
>
> Signed-off-by: Biju Das 

After reading through context this seems like the right fix so patch applied!

In the long run I want to get rid of this extra of_node in the chip, but it
kind of requires every driver in the kernel to pass in a valid parent device
representing the OF node. And there are so many hairy corner cases
where they don't.

Yours,
Linus Walleij


Re: [PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support

2018-08-06 Thread Kieran Bingham
Hi Eugeniu

On 05/08/18 00:11, Eugeniu Rosca wrote:
> After adding CAN support to arch/arm64/boot/dts/renesas/r8a77965.dtsi,
> checkpatch complained that the new compatible string
> "renesas,can-r8a77965" is not documented. Fix the warning.
> 

Thanks to the correct ordering of your patches, (you have this one
*before* adding the CAN support to r8a77965) This commit message seems
to be predicting the future somewhat.

Perhaps just a simpler commit message would suffice:

"Document the support for rcar_can on R8A77965 SoC devices."


> Signed-off-by: Eugeniu Rosca 
> ---
>  Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt 
> b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> index 94a7f33ac5e9..23264451a5a4 100644
> --- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
> @@ -13,6 +13,7 @@ Required properties:
> "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
> "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
> "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
> +   "renesas,can-r8a77965" if CAN controller is a part of R8A77965 
> SoC.
> "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible 
> device.
> "renesas,rcar-gen2-can" for a generic R-Car Gen2 or RZ/G1
> compatible device.
> @@ -28,9 +29,8 @@ Required properties:
>  - pinctrl-0: pin control group to be used for this controller.
>  - pinctrl-names: must be "default".
>  
> -Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
> -compatible:
> -In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
> +Required properties for compatibles [A], [B] and [C]:
> +For the denoted SoCs, "clkp2" can be CANFD clock. This is a div6 clock


This paragraph could be rewrapped...

>  and can be used by both CAN and CAN FD controller at the same time. It needs 
> to
>  be scaled to maximum frequency if any of these controllers use it. This is 
> done
>  using the below properties:
> @@ -38,6 +38,10 @@ using the below properties:
>  - assigned-clocks: phandle of clkp2(CANFD) clock.
>  - assigned-clock-rates: maximum frequency of this clock.
>  
> +[A] "renesas,can-r8a7795"
> +[B] "renesas,can-r8a7796"
> +[C] "renesas,can-r8a77965"
> +>  Optional properties:
>  - renesas,can-clock-select: R-Car CAN Clock Source Select. Valid values are:
>   <0x0> (default) : Peripheral clock (clkp1)
> 



Re: [PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible

2018-08-06 Thread Kieran Bingham
On 06/08/18 11:45, Kieran Bingham wrote:
> Hi Laurent, Eugeniu,
> 
> On 06/08/18 11:42, Laurent Pinchart wrote:
>> Hi Eugeniu,
>>
>> Thank you for the patch.
>>
>> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote:
>>> Following the recent change in dt-bindings [1], switch from
>>> "renesas,h3ulcb" to "renesas,ulcb" compatible string.
>>>
>>> [1] Documentation/devicetree/bindings/arm/shmobile.txt
>>>
>>> Signed-off-by: Eugeniu Rosca 
>>> ---
>>>  arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index
>>> 06deb67c36c8..7a5b1dc64090 100644
>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>>> @@ -14,6 +14,6 @@
>>>
>>>  / {
>>> model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x";
>>> -   compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
>>> +   compatible = "shimafuji,kingfisher", "renesas,ulcb",
>>>  "renesas,r8a7795";
>>
>> This is unrelated to your patch, but due to the reason explained in my 
>> review 
>> of 02/14, I think "shimafuji,kingfisher" should include the SoC name.
>>
>> This brings up the topic of how to describe boards that are made of an SoC 
>> "module" board plugged into an expansion "motherboard".
> 
> Isn't it the point that the shimafuji board is agnostic to the SoC on
> the ULCB?
> 
> I presume the Kingfisher board is just the expansion board which would
> be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB?


In fact possibly the interesting point is that the kingfisher as an
expansion board is surely an 'overlay', rather than the board itself ?

>>
>>>  };


Regards

Kieran



Re: [PATCH v2 2/5] dt-bindings: gpio: rcar: Add gpio-reserved-ranges support

2018-08-06 Thread Linus Walleij
Hi Biju!

Thanks for the patches and sorry for slow feedback.
Luckily you have Geert to help out and I can see this is already
getting in nice shape.

On Thu, Aug 2, 2018 at 4:17 PM Biju Das  wrote:

> Update the DT bindings documentation with the optional gpio-reserved-ranges
> properties.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
(...)
> +  - gpio-reserved-ranges: Set of tuples to specify the unused GPIOs.This
> +property indicates the start and size of the GPIOs that can't be used.
> +
>  Please refer to gpio.txt in this directory for details of gpio-ranges 
> property
>  and the common GPIO bindings used by client devices.

You see it yourself if you look at the context below: just refer to the existing
documentation in gpio.txt as this is a standard bindings.

The absolutely best (IIUC) is:

gpio-ranges: See gpio.txt
gpio-reserved-ranges: See gpio.txt

Maybe you could add an extra example using the reserved ranges?

Yours,
Linus Walleij


Re: [PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible

2018-08-06 Thread Kieran Bingham
Hi Laurent, Eugeniu,

On 06/08/18 11:42, Laurent Pinchart wrote:
> Hi Eugeniu,
> 
> Thank you for the patch.
> 
> On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote:
>> Following the recent change in dt-bindings [1], switch from
>> "renesas,h3ulcb" to "renesas,ulcb" compatible string.
>>
>> [1] Documentation/devicetree/bindings/arm/shmobile.txt
>>
>> Signed-off-by: Eugeniu Rosca 
>> ---
>>  arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index
>> 06deb67c36c8..7a5b1dc64090 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
>> @@ -14,6 +14,6 @@
>>
>>  / {
>>  model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x";
>> -compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
>> +compatible = "shimafuji,kingfisher", "renesas,ulcb",
>>   "renesas,r8a7795";
> 
> This is unrelated to your patch, but due to the reason explained in my review 
> of 02/14, I think "shimafuji,kingfisher" should include the SoC name.
> 
> This brings up the topic of how to describe boards that are made of an SoC 
> "module" board plugged into an expansion "motherboard".

Isn't it the point that the shimafuji board is agnostic to the SoC on
the ULCB?

I presume the Kingfisher board is just the expansion board which would
be identical regardless of if it was put on an H3 ULCB, or an M3 ULCB?


> 
>>  };
> 

Regards

Kieran


[PATCH] ata: sata_rcar: exclude setting of PHY registers in Gen3

2018-08-06 Thread Wolfram Sang
From: Masaharu Hayakawa 

According to documentation, setting of PHY registers is unnecessary with
R-Car Gen3. The registers are not even described. So, don't initialize
them.

Signed-off-by: Masaharu Hayakawa 
[wsa: updated commit message]
Signed-off-by: Wolfram Sang 
---

Tested with a Renesas R-Car M3N Salvator-XS board. I did a platform suspend to
check if the controller and PHY work properly after resume.

 drivers/ata/sata_rcar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 1ad168f76ef3..10ecb232245d 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -830,10 +830,11 @@ static void sata_rcar_init_controller(struct ata_host 
*host)
sata_rcar_gen1_phy_init(priv);
break;
case RCAR_GEN2_SATA:
-   case RCAR_GEN3_SATA:
case RCAR_R8A7790_ES1_SATA:
sata_rcar_gen2_phy_init(priv);
break;
+   case RCAR_GEN3_SATA:
+   break;
default:
dev_warn(host->dev, "SATA phy is not initialized\n");
break;
@@ -995,7 +996,6 @@ static int sata_rcar_resume(struct device *dev)
return ret;
 
if (priv->type == RCAR_GEN3_SATA) {
-   sata_rcar_gen2_phy_init(priv);
sata_rcar_init_module(priv);
} else {
/* ack and mask */
-- 
2.11.0



Re: [PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible

2018-08-06 Thread Laurent Pinchart
Hi Eugeniu,

Thank you for the patch.

On Sunday, 5 August 2018 02:11:04 EEST Eugeniu Rosca wrote:
> Following the recent change in dt-bindings [1], switch from
> "renesas,h3ulcb" to "renesas,ulcb" compatible string.
> 
> [1] Documentation/devicetree/bindings/arm/shmobile.txt
> 
> Signed-off-by: Eugeniu Rosca 
> ---
>  arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts index
> 06deb67c36c8..7a5b1dc64090 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> @@ -14,6 +14,6 @@
> 
>  / {
>   model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES1.x";
> - compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
> + compatible = "shimafuji,kingfisher", "renesas,ulcb",
>"renesas,r8a7795";

This is unrelated to your patch, but due to the reason explained in my review 
of 02/14, I think "shimafuji,kingfisher" should include the SoC name.

This brings up the topic of how to describe boards that are made of an SoC 
"module" board plugged into an expansion "motherboard".

>  };

-- 
Regards,

Laurent Pinchart





[PATCH] ata: sata_rcar: really mask all interrupts on Gen2 and later

2018-08-06 Thread Wolfram Sang
Since R-Car Gen2, a new bit has been introduced to the interrupt mask
register. Update the code to handle it properly as well.

Signed-off-by: Wolfram Sang 
---

Based on a report and prototype from the BSP team.

 drivers/ata/sata_rcar.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 3b8ed10bfb8f..1ad168f76ef3 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -109,6 +109,8 @@
 #define SATAINTMASK_ERRMSK BIT(2)
 #define SATAINTMASK_ERRCRTMSK  BIT(1)
 #define SATAINTMASK_ATAMSK BIT(0)
+#define SATAINTMASK_ALL_GEN1   0x7ff
+#define SATAINTMASK_ALL_GEN2   0xfff
 
 #define SATA_RCAR_INT_MASK (SATAINTMASK_SERRMSK | \
 SATAINTMASK_ATAMSK)
@@ -152,6 +154,7 @@ enum sata_rcar_type {
 
 struct sata_rcar_priv {
void __iomem *base;
+   u32 sataint_mask;
enum sata_rcar_type type;
 };
 
@@ -225,7 +228,7 @@ static void sata_rcar_freeze(struct ata_port *ap)
struct sata_rcar_priv *priv = ap->host->private_data;
 
/* mask */
-   iowrite32(0x7ff, priv->base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask, priv->base + SATAINTMASK_REG);
 
ata_sff_freeze(ap);
 }
@@ -241,7 +244,7 @@ static void sata_rcar_thaw(struct ata_port *ap)
ata_sff_thaw(ap);
 
/* unmask */
-   iowrite32(0x7ff & ~SATA_RCAR_INT_MASK, base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask & ~SATA_RCAR_INT_MASK, base + 
SATAINTMASK_REG);
 }
 
 static void sata_rcar_ioread16_rep(void __iomem *reg, void *buffer, int count)
@@ -735,7 +738,7 @@ static irqreturn_t sata_rcar_interrupt(int irq, void 
*dev_instance)
if (!sataintstat)
goto done;
/* ack */
-   iowrite32(~sataintstat & 0x7ff, base + SATAINTSTAT_REG);
+   iowrite32(~sataintstat & priv->sataint_mask, base + SATAINTSTAT_REG);
 
ap = host->ports[0];
 
@@ -808,7 +811,7 @@ static void sata_rcar_init_module(struct sata_rcar_priv 
*priv)
 
/* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
-   iowrite32(0x7ff, base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
 
/* enable interrupts */
iowrite32(ATAPI_INT_ENABLE_SATAINT, base + ATAPI_INT_ENABLE_REG);
@@ -818,9 +821,12 @@ static void sata_rcar_init_controller(struct ata_host 
*host)
 {
struct sata_rcar_priv *priv = host->private_data;
 
+   priv->sataint_mask = SATAINTMASK_ALL_GEN2;
+
/* reset and setup phy */
switch (priv->type) {
case RCAR_GEN1_SATA:
+   priv->sataint_mask = SATAINTMASK_ALL_GEN1;
sata_rcar_gen1_phy_init(priv);
break;
case RCAR_GEN2_SATA:
@@ -948,7 +954,7 @@ static int sata_rcar_remove(struct platform_device *pdev)
iowrite32(0, base + ATAPI_INT_ENABLE_REG);
/* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
-   iowrite32(0x7ff, base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
 
pm_runtime_put(>dev);
pm_runtime_disable(>dev);
@@ -969,7 +975,7 @@ static int sata_rcar_suspend(struct device *dev)
/* disable interrupts */
iowrite32(0, base + ATAPI_INT_ENABLE_REG);
/* mask */
-   iowrite32(0x7ff, base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
 
pm_runtime_put(dev);
}
@@ -994,7 +1000,7 @@ static int sata_rcar_resume(struct device *dev)
} else {
/* ack and mask */
iowrite32(0, base + SATAINTSTAT_REG);
-   iowrite32(0x7ff, base + SATAINTMASK_REG);
+   iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
 
/* enable interrupts */
iowrite32(ATAPI_INT_ENABLE_SATAINT,
-- 
2.11.0



Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible

2018-08-06 Thread Laurent Pinchart
Hi Eugeniu,

Thank you for the patch.

On Sunday, 5 August 2018 02:11:02 EEST Eugeniu Rosca wrote:
> In the context of M3N-ULCB (RTP0RC77965SKBX010SA00) board bring-up, it's
> rather pointless to add a new "renesas,m3nulcb" compatible string. Any
> SoC-level differences between the two variants of ULCB (M3 and M3-N)
> should be successfully covered by making use of existing
> "renesas,r8a7796" and "renesas,r8a77965" compatibles.
> 
> Prior to adding M3-N Starter Kit to the list, rename:
>  - "renesas,h3ulcb" => "renesas,ulcb"
>  - "renesas,m3ulcb" => "renesas,ulcb"

This bothers me more than the naming convention in patch 01/14, as this change 
would completely hide differences between the H3 and M3-N versions of the 
ULCB. Compatible strings are listed in a decreasing order of specificity, and 
having "renesas,ulcb" as the most-specific compatible string means that the 
two boards are supposed to be identical, while they are not.

> Relevant DTS changes come in separate per-DTS commits.
> 
> Signed-off-by: Eugeniu Rosca 
> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt
> b/Documentation/devicetree/bindings/arm/shmobile.txt index
> d8cf740132c6..f391dba10574 100644
> --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -81,7 +81,7 @@ Boards:
>  compatible = "renesas,gose", "renesas,r8a7793"
>- H3ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
>  H3ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
> -compatible = "renesas,h3ulcb", "renesas,r8a7795"
> +compatible = "renesas,ulcb", "renesas,r8a7795"
>- Henninger
>  compatible = "renesas,henninger", "renesas,r8a7791"
>- iWave Systems RZ/G1C Single Board Computer (iW-RainboW-G23S)
> @@ -105,7 +105,7 @@ Boards:
>- Lager (RTP0RC7790SEB00010S)
>  compatible = "renesas,lager", "renesas,r8a7790"
>- M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
> -compatible = "renesas,m3ulcb", "renesas,r8a7796"
> +compatible = "renesas,ulcb", "renesas,r8a7796"
>- Marzen (R0P7779A00010S)
>  compatible = "renesas,marzen", "renesas,r8a7779"
>- Porter (M2-LCDP)

-- 
Regards,

Laurent Pinchart





Re: [PATCH 01/14] arm64: dts: renesas: Cut redundant in *-ulcb*.dts

2018-08-06 Thread Laurent Pinchart
Hello Eugeniu,

Thank you for the patch.

On Sunday, 5 August 2018 02:11:01 EEST Eugeniu Rosca wrote:
> Perform a 's/(h|m)3ulcb/ulcb/' substitution in below DTS filenames:
>  - r8a7795-es1-h3ulcb.dts=> r8a7795-es1-ulcb.dts
>  - r8a7795-es1-h3ulcb-kf.dts => r8a7795-es1-ulcb-kf.dts
>  - r8a7795-h3ulcb.dts=> r8a7795-ulcb.dts
>  - r8a7795-h3ulcb-kf.dts => r8a7795-ulcb-kf.dts
>  - r8a7796-m3ulcb.dts=> r8a7796-ulcb.dts
>  - r8a7796-m3ulcb-kf.dts => r8a7796-ulcb-kf.dts
> 
> The background of this commit is M3-N ULCB (RTP0RC77965SKBX010SA00)
> bring-up, which (assuming no change in existing DTS name patterns)
> requires two new DTS files:
>  - r8a77965-m3nulcb.dts
>  - r8a77965-m3nulcb-kf.dts
> 
> In all above examples:
>  - "m3n" prefix is redundant, since r8a77965 denotes the M3-N SoC
>  - "m3"  prefix is redundant, since r8a7796  denotes the M3/M3-W SoC
>  - "h3"  prefix is redundant, since r8a7795  denotes the H3 SoC

The naming convention is roughly -.dts. For instance, in r8a7795-
h3ulcb.dts, the SoC is R8A7795 and the board "H3 ULCB". With the proposed 
rename we would break that convention.

However, the name ULCB itself (which stands for Ultra Low Cost Board) might 
already not follow the naming convention, as the boards are officially called 
R-Car Starter Kit (Pro and Premier). The V3M and V3H "low-cost" boards reflect 
that properly, with their .dts files named r8a77970-v3msk.dts and r8a77970-
v3hsk.dts respectively.

I'm not opposed to simplifying the file names, but I think we should then 
decide on a simpler convention. In particular the H3/M3 and V3 .dts files 
should in my opinion follow the same convention.

I'll now let others comment on this as I don't have such a strong opinion on 
this topic.

> To make the DTS naming conventions consistent, drop the unneeded
> prefixes. Similar reasoning was applied by Marek in U-Boot v2017.09
> commit bd39050cb2a0 ("ARM: rmobile: ulcb: Add ULCB board support"):
> 
> $ git log -1 --format= --name-only bd39050cb2a -- "*defconfig"
> configs/r8a7795_ulcb_defconfig
> configs/r8a7796_ulcb_defconfig
> 
> DTB md5 sums match with and w/o the patch (hence no functional change).
> 
> Signed-off-by: Eugeniu Rosca 
> ---
>  arch/arm64/boot/dts/renesas/Makefile   
>| 12 ++--
> arch/arm64/boot/dts/renesas/{r8a7795-es1-h3ulcb-kf.dts =>
> r8a7795-es1-ulcb-kf.dts} |  2 +-
> arch/arm64/boot/dts/renesas/{r8a7795-es1-h3ulcb.dts =>
> r8a7795-es1-ulcb.dts}   |  0
> arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb-kf.dts => r8a7795-ulcb-kf.dts} 
>|  2 +- arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb.dts =>
> r8a7795-ulcb.dts}   |  0
> arch/arm64/boot/dts/renesas/{r8a7796-m3ulcb-kf.dts => r8a7796-ulcb-kf.dts} 
>|  2 +- arch/arm64/boot/dts/renesas/{r8a7796-m3ulcb.dts =>
> r8a7796-ulcb.dts}   |  0 7 files changed, 9 insertions(+), 9
> deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile
> b/arch/arm64/boot/dts/renesas/Makefile index 9e2394bc3c62..5debb02fad2c
> 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -1,11 +1,11 @@
>  # SPDX-License-Identifier: GPL-2.0
> -dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-h3ulcb.dtb
> -dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-h3ulcb-kf.dtb
> +dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb r8a7795-ulcb.dtb
> +dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-ulcb-kf.dtb
>  dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-xs.dtb
> -dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-es1-salvator-x.dtb
> r8a7795-es1-h3ulcb.dtb -dtb-$(CONFIG_ARCH_R8A7795) +=
> r8a7795-es1-h3ulcb-kf.dtb
> -dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-x.dtb r8a7796-m3ulcb.dtb
> -dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-m3ulcb-kf.dtb
> +dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-es1-salvator-x.dtb
> r8a7795-es1-ulcb.dtb +dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-es1-ulcb-kf.dtb
> +dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-x.dtb r8a7796-ulcb.dtb
> +dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-ulcb-kf.dtb
>  dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-xs.dtb
>  dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-x.dtb
> r8a77965-salvator-xs.dtb dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb
> r8a77970-v3msk.dtb diff --git
> a/arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb-kf.dts
> b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts similarity index 93%
> rename from arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb-kf.dts
> rename to arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> index 009cb1cb0dde..06deb67c36c8 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb-kf.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb-kf.dts
> @@ -9,7 +9,7 @@
>   * kind, whether express or implied.
>   */
> 
> -#include "r8a7795-es1-h3ulcb.dts"
> +#include "r8a7795-es1-ulcb.dts"
>  #include "ulcb-kf.dtsi"
> 
>  / {
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb.dts
> 

[PATCH] gpiolib: Fix of_node inconsistency

2018-08-06 Thread Biju Das
Some platforms are not setting of_node in the driver. On these platforms
defining gpio-reserved-ranges on device tree leads to kernel crash.

It is due to some parts of the gpio core relying on the driver to set up
of_node,while other parts do themselves.This inconsistent behaviour leads 
to a crash.

gpiochip_add_data_with_key() calls gpiochip_init_valid_mask() with of_node
as NULL. of_gpiochip_add() fills "of_node" and calls 
of_gpiochip_init_valid_mask().

The fix is to move the assignment to chip->of_node from of_gpiochip_add()
to gpiochip_add_data_with_key().

Signed-off-by: Biju Das 
---
 drivers/gpio/gpiolib-of.c | 3 ---
 drivers/gpio/gpiolib.c| 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 28d9680..91174bf 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -620,9 +620,6 @@ int of_gpiochip_add(struct gpio_chip *chip)
 {
int status;
 
-   if ((!chip->of_node) && (chip->parent))
-   chip->of_node = chip->parent->of_node;
-
if (!chip->of_node)
return 0;
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb..20fe531 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1256,6 +1256,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, 
void *data,
/* If the gpiochip has an assigned OF node this takes precedence */
if (chip->of_node)
gdev->dev.of_node = chip->of_node;
+   else
+   chip->of_node = gdev->dev.of_node;
 #endif
 
gdev->id = ida_simple_get(_ida, 0, 0, GFP_KERNEL);
-- 
2.7.4



Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible

2018-08-06 Thread Eugeniu Rosca
Hi Morimoto-san,

Thank you for your comments.

On Mon, Aug 06, 2018 at 12:33:34AM +, Kuninori Morimoto wrote:
> Hi Eugeniu
> 
> > In the context of M3N-ULCB (RTP0RC77965SKBX010SA00) board bring-up, it's
> > rather pointless to add a new "renesas,m3nulcb" compatible string. Any
> > SoC-level differences between the two variants of ULCB (M3 and M3-N)
> > should be successfully covered by making use of existing
> > "renesas,r8a7796" and "renesas,r8a77965" compatibles.
> > 
> > Prior to adding M3-N Starter Kit to the list, rename:
> >  - "renesas,h3ulcb" => "renesas,ulcb"
> >  - "renesas,m3ulcb" => "renesas,ulcb"
> 
> I'm not sure detail, but
> does it mean, both H3/M3 board can boot/work with
> "renesas,ulcb" compatible if we had such driver/soc ?

First, assuming latest vanilla v4.18-rc8 kernel, neither
"renesas,salvator-x[s]" nor "renesas,(m3|h3)ulcb" compatibles are
used anywhere outside of DTS and DT bindings documentation:

$ git grep -E --name-only "renesas,(h3ulcb|m3ulcb|salvator-x)" | xargs dirname 
| sort -u
arch/arm64/boot/dts/renesas
Documentation/devicetree/bindings/arm

Since there is no driver using these compatibles, no functional
breakage is expected by changing the compatible format/name.

Secondly, as pointed out in the commit summary line and description,
there is an overlap in scope between the SoC-level compatibles and
ULCB board-level compatibles, which doesn't happen for Salvator-X{S}
targets and creates some inconsistency. This inconsistency now spawns
debates about how other ULCB-based board compatibles should be named and
for that single reason IMO should be fixed.

Lastly, I don't think any driver will ever need to use
"renesas,(m3|m3n|h3)ulcb" string, since it is too broad. On/off-chip
IP-oriented compatibles are probably better candidates for that.

> 
> > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt 
> > b/Documentation/devicetree/bindings/arm/shmobile.txt
> > index d8cf740132c6..f391dba10574 100644
> > --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> > @@ -81,7 +81,7 @@ Boards:
> >  compatible = "renesas,gose", "renesas,r8a7793"
> >- H3ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
> >  H3ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
> > -compatible = "renesas,h3ulcb", "renesas,r8a7795"
> > +compatible = "renesas,ulcb", "renesas,r8a7795"
> >- Henninger
> >  compatible = "renesas,henninger", "renesas,r8a7791"
> >- iWave Systems RZ/G1C Single Board Computer (iW-RainboW-G23S)
> > @@ -105,7 +105,7 @@ Boards:
> >- Lager (RTP0RC7790SEB00010S)
> >  compatible = "renesas,lager", "renesas,r8a7790"
> >- M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
> > -compatible = "renesas,m3ulcb", "renesas,r8a7796"
> > +compatible = "renesas,ulcb", "renesas,r8a7796"
> >- Marzen (R0P7779A00010S)
> >  compatible = "renesas,marzen", "renesas,r8a7779"
> >- Porter (M2-LCDP)
> 
> My opinion is that if you want to exchange compatible name,
> related all driver/document should be exchanged in same patch.

AFAIK Simon maintains a number of branches hosting solely the DT
bindings. More precisely it is the "dt-bindings-for-v4.*" branch series
in https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git

For that reason, IMO it might be worth to detach the document updates
from DTS updates. I have no problems squashing the DTS and doc patches
into one single commit, but before doing that I would appreciate a
confirmation from the maintainer. Anyhow, many thanks for your feedback!

> 
> For example, above "h3ulcb" case, patch subject indicates
> "rename h3ulcb" or something, and it include [03/14][04/14][05/14][06/14].
> Same for m3

It was my impression that the DTS patches are always partitioned
per-file, to avoid misleading globbing patterns in the commit subjects
and allow easier DTS commit porting to future SoCs/boards. I will
gladly follow your suggestion once I get the confirmation from
maintainer.

Thank you very much!

Best regards,
Eugeniu.


RE: [PATCH v2 1/5] gpio: rcar: Add GPIO hole support

2018-08-06 Thread Biju Das
HI Geert,

Thanks for the feedback.

> -Original Message-
> From: Geert Uytterhoeven 
> Sent: 04 August 2018 10:25
> To: Biju Das 
> Cc: Linus Walleij ; open list:GPIO SUBSYSTEM
> ; Simon Horman ;
> Geert Uytterhoeven ; Chris Paterson
> ; Fabrizio Castro
> ; Linux-Renesas  s...@vger.kernel.org>
> Subject: Re: [PATCH v2 1/5] gpio: rcar: Add GPIO hole support
> > > > @@ -471,6 +495,7 @@ static int gpio_rcar_probe(struct
> > > > platform_device
> > > *pdev)
> > > > gpio_chip->owner = THIS_MODULE;
> > > > gpio_chip->base = -1;
> > > > gpio_chip->ngpio = npins;
> > > > +   gpio_chip->need_valid_mask = p->bank_info.gpiomsk ? true :
> > > > + false;
> > >
> > > gpiochip_init_valid_mask() already sets gpio_chip->need_valid_mask
> > > to true if "gpio-reserved-ranges" is detected.
> >
> > The plan is  to set  "of_node"  variable in driver. So that
> gpiochip_init_valid_mask() sets gpio_chip->need_valid_mask to true.
> > What is your opinion on this?
>
> Sounds fine to me.
>
> You said on IRC that it currently crashes when "gpio-reserved-ranges" is
> used.

Yes that is correct. if we add "gpio-reserved-ranges"  on device tree without
setting  "of_node" or  " need_valid_mask"  in driver, gpiochip_init_valid_mask()
won't allocate memory for valid_mask and  bitmap_clear() in 
of_gpiochip_init_valid_mask()
crashes. May be we need to modify the code like this to fix the crash.

if (chip->valid_mask)
bitmap_clear(chip->valid_mask, start, count);
else
pr_warn("valid_mask is not set ");



> IMHO that's something that should be fixed separately, possibly for v4.19.
>
> Looks like all other Renesas gpio drivers (some combined pinctrl/gpio),
> except for gpio-em.c, also fail to set gpio_chip.of_node. Will have a closer
> look...

Regards,
Biju



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.