Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
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
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
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
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
> + !(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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> "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
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"
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
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
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
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"
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
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
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"
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
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
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
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
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"
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
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
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"
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.