Re: [PATCH/RFC 17/19] arm64: dts: renesas: Add support for Salvator-XS with R-Car M3-W+
Hi Geert,
On Wed, Oct 16, 2019 at 10:54:12AM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
>
> On Mon, Oct 14, 2019 at 7:57 PM Eugeniu Rosca wrote:
> > On Mon, Oct 07, 2019 at 12:23:30PM +0200, Geert Uytterhoeven wrote:
> > > Add initial support for the Renesas Salvator-X 2nd version development
> > > board equipped with an R-Car M3-W+ SiP with 8 (2 x 4) GiB of RAM.
> > >
> > > The memory map is as follows:
> > > - Bank0: 4GiB RAM : 0x4800 -> 0x000bfff
> > > 0x00048000 -> 0x004
> > > - Bank1: 4GiB RAM : 0x0006 -> 0x006
> > >
> > > Based on a patch in the BSP by Takeshi Kihara
> > > .
> > >
> > > Signed-off-by: Geert Uytterhoeven
> > > ---
> > > arch/arm64/boot/dts/renesas/Makefile | 1 +
> > > .../boot/dts/renesas/r8a77961-salvator-xs.dts | 31 +++
> > > 2 files changed, 32 insertions(+)
> > > create mode 100644 arch/arm64/boot/dts/renesas/r8a77961-salvator-xs.dts
> >
> > It is common practice in Renesas BSP to specify the SiP memory
> > split by suffixing the DTB names with '-{2,4}x{2,4}g' [1].
> >
> > Has this ever been discussed on ML?
> >
> > Here in particular, it would allow M3-W+ 2x4GiB Salvator-XS and
> > M3-W+ 2x2GiB (or any other DRAM split flavor of) Salvator-XS to
> > coexist in harmony, if the latter pops up at any point.
>
> With mainline U-Boot, the memory configuration is passed from ATF
> through U-Boot to Linux, see e.g. "ARM: renesas: Configure DRAM size
> from ATF DT fragment" [1], so there's no longer a need to maintain
> multiple DTS files.
With CONFIG_ARCH_FIXUP_FDT_MEMORY being disabled on most, if not all,
R-Car3 targets in u-boot master [2], it's unlikely we'll get any DRAM
information passed via DT from U-Boot to Linux.
I notice that Marek (CC) has just submitted a patch to re-enable [3]
the U-Boot feature. Does this mean the community is fine with the idea
that adjusting the Linux DT memory entries (e.g. for debugging and
other purposes) will become a NOOP and will require users to reflash
their bootloaders?
> [1]
> https://gitlab.denx.de/u-boot/u-boot/commit/175f5027345c7feaa41e8f4201778814bf72fe37
[2] u-boot (6891152a4596) git grep FIXUP_FDT -- configs/r8a779*
configs/r8a7795_salvator-x_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a7795_ulcb_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a77965_salvator-x_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a77965_ulcb_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a7796_salvator-x_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a7796_ulcb_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a77970_eagle_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a77990_ebisu_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
configs/r8a77995_draak_defconfig:# CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
[3] https://patchwork.ozlabs.org/patch/1177387/
("ARM: rmobile: Enable CONFIG_ARCH_FIXUP_FDT_MEMORY on Gen3")
--
Best Regards,
Eugeniu
Re: [PATCH/RFC 00/19] arm64: dts: renesas: Initial support for R-Car M3-W+
Hi Geert, On Mon, Oct 07, 2019 at 12:23:13PM +0200, Geert Uytterhoeven wrote: > Hi all, > > This RFC patch series adds support for the R-Car M3-W+ (R8A77961) SoC > and the Salvator-XS board with R-Car M3-W+. This SoC is a derivative of > R-Car M3-W (R8A77960), and also known as R-Car M3-W ES3.0. > As this is an RFC, I'm sending it to a limited audience. > > Based on experience with previous SoCs in the R-Car Gen3 family, the > following design decisions were made: > - Use different compatible values (r8a77961-based), Given that a potentially incomplete list of M3-W compatible strings counts 40 occurrences [1] and this series adds only 7 [2], current RFC looks like the first step in a multi-phase approach. Do you plan to add the missing r8a77961 compatibles in the next revision or do you expect other people to contribute those later? > - Use different clock and SYSC DT binding definitions > (R8A77961-based), but the same numerical values, to allow sharing > drivers, > - Share the pin control driver, > - Share the clock driver, > - Share the system controller driver. > > While the DT ABI is stable (hence we cannot s/r8a7796/r8a77960/ in DTS), > kernel source code and kernel config symbols can be changed at any > time. As changing kernel config symbols impacts the user, they weren't > renamed yet. > > Questions: > - What's the board part number of Salvator-XS with R-Car M3-W+? I guess my board is an exception, since it got the SiP simply upgraded from SoC ES1.x to ES3.0 by resoldering. IOW the board carries the same serial number as M3-ES1.1 Salvator-XS. [..] > - Should the R8A77961 config symbols be dropped? > - CONFIG_ARCH_R8A77961 > - CONFIG_CLK_R8A77961 > - CONFIG_PINCTRL_PFC_R8A77961 > - CONFIG_SYSC_R8A77961 > > - If not, should the R8A7796 config symbols be renamed? > - CONFIG_ARCH_R8A7796 to CONFIG_ARCH_R8A77960? > - CONFIG_CLK_R8A7796 to CONFIG_CLK_R8A77960? > - CONFIG_PINCTRL_PFC_R8A7796 to CONFIG_PINCTRL_PFC_R8A77960? > - CONFIG_SYSC_R8A7796 to CONFIG_SYSC_R8A77960? > Due to dependencies on CONFIG_ARCH_R8A7796, this should be a single > commit. [2 cents] Both adding CONFIG_*_R8A77961 and renaming CONFIG_*_R8A7796 to CONFIG_*_R8A77960 make sense to me. > Related questions for old R-Car H3 ES1.x support: > - Should CONFIG_PINCTRL_PFC_R8A77950 be added, to allow compiling out > R-Car H3 ES1.x pin control support? [2 cents] Adding CONFIG_*_R8A77950 makes sense in spite of the fact that R8A77950 is not documented in R-Car HW man. In fact, it is quite clear why R8A77950 is _not_ documented while R8A77960 _is_ documented. The former is obsolete (the community is nice by not obliterating its support) while the latter is expected to hit the market. > If yes, should CONFIG_PINCTRL_PFC_R8A7795 be renamed to > CONFIG_PINCTRL_PFC_R8A77951? In a perfect/ideal world, I would say yes. > This patch series is based on renesas-drivers-2019-10-01-v5.4-rc1). For > your convenience, it is available in the topic/r8a77961-v1 branch of my > renesas-drivers git repository at > git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git. > > This has been tested using remote access. > > Thanks for your comments! Thanks for the bring-up! [..] [1] linux (geert-renesas-drivers/topic/r8a77961-v1) \ git grep -o "\"renesas[^\ ]*r8a7796\b[^\"]*\"" | cut -f2 -d: | sort -u "renesas,can-r8a7796" "renesas,dmac-r8a7796" "renesas,du-r8a7796" "renesas,etheravb-r8a7796" "renesas,gpio-r8a7796" "renesas,hscif-r8a7796" "renesas,i2c-r8a7796" "renesas,iic-r8a7796" "renesas,intc-ex-r8a7796" "renesas,ipmmu-r8a7796" "renesas,msiof-r8a7796" "renesas,pcie-r8a7796" "renesas,pfc-r8a7796" "renesas,pwm-r8a7796" "renesas,r8a7796" "renesas,r8a7796-canfd" "renesas,r8a7796-cmt0" "renesas,r8a7796-cmt1" "renesas,r8a7796-cpg-mssr" "renesas,r8a7796-csi2" "renesas,r8a7796-drif" "renesas,r8a7796-hdmi" "renesas,r8a7796-imr-lx4" "renesas,r8a7796-lvds" "renesas,r8a7796-rcar-usb2-clock-sel" "renesas,r8a7796-rst" "renesas,r8a7796-sysc" "renesas,r8a7796-thermal" "renesas,r8a7796-usb3-peri" "renesas,r8a7796-usb3-phy" "renesas,r8a7796-usb-dmac" "renesas,r8a7796-wdt" "renesas,rcar_sound-r8a7796" "renesas,scif-r8a7796" "renesas,sdhi-r8a7796" "renesas,tpu-r8a7796" "renesas,usb2-phy-r8a7796" "renesas,usbhs-r8a7796" "renesas,vin-r8a7796" "renesas,xhci-r8a7796" [2] linux (geert-renesas-drivers/topic/r8a77961-v1) \ git grep -o "\"renesas[^\ ]*r8a77961\b[^\"]*\"" | cut -f2 -d: | sort -u "renesas,hscif-r8a77961" "renesas,pfc-r8a77961" "renesas,r8a77961" "renesas,r8a77961-cpg-mssr" "renesas,r8a77961-rst" "renesas,r8a77961-sysc" "renesas,scif-r8a77961" -- Best Regards, Eugeniu
Re: [PATCH/RFC 03/19] dt-bindings: clock: renesas: cpg-mssr: Document r8a77961 support
On Mon, Oct 07, 2019 at 12:23:16PM +0200, Geert Uytterhoeven wrote: > Add DT binding documentation for the Clock Pulse Generator / Module > Standby and Software Reset block in the Renesas R-Car M3-W+ (R8A77961) > SoC. > > Signed-off-by: Geert Uytterhoeven > --- > .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > index b5edebeb12b40638..b9b0927b7c780699 100644 > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt [..] > @@ -42,10 +43,10 @@ Required Properties: >- clock-names: List of external parent clock names. Valid names are: >- "extal" (r7s9210, r8a7743, r8a7744, r8a7745, r8a77470, r8a774a1, >r8a774b1, r8a774c0, r8a7790, r8a7791, r8a7792, r8a7793, > - r8a7794, r8a7795, r8a7796, r8a77965, r8a77970, r8a77980, > - r8a77990, r8a77995) > - - "extalr" (r8a774a1, r8a774b1, r8a7795, r8a7796, r8a77965, r8a77970, > - r8a77980) > + r8a7794, r8a7795, r8a7796, r8a77961, r8a77965, r8a77970, > + r8a77980, r8a77990, r8a77995) > + - "extalr" (r8a774a1, r8a774b1, r8a7795, r8a7796, r8a77961, r8a77965, > + r8a77970, r8a77980) >- "usb_extal" (r8a7743, r8a7744, r8a7745, r8a77470, r8a7790, r8a7791, >r8a7793, r8a7794) Not easy to review, but 'git show --color-words' comes to the rescue :) -- Best Regards, Eugeniu
Re: [PATCH/RFC 17/19] arm64: dts: renesas: Add support for Salvator-XS with R-Car M3-W+
Hi Geert,
Thanks for the series. One question below.
On Mon, Oct 07, 2019 at 12:23:30PM +0200, Geert Uytterhoeven wrote:
> Add initial support for the Renesas Salvator-X 2nd version development
> board equipped with an R-Car M3-W+ SiP with 8 (2 x 4) GiB of RAM.
>
> The memory map is as follows:
> - Bank0: 4GiB RAM : 0x4800 -> 0x000bfff
> 0x00048000 -> 0x004
> - Bank1: 4GiB RAM : 0x0006 -> 0x006
>
> Based on a patch in the BSP by Takeshi Kihara
> .
>
> Signed-off-by: Geert Uytterhoeven
> ---
> arch/arm64/boot/dts/renesas/Makefile | 1 +
> .../boot/dts/renesas/r8a77961-salvator-xs.dts | 31 +++
> 2 files changed, 32 insertions(+)
> create mode 100644 arch/arm64/boot/dts/renesas/r8a77961-salvator-xs.dts
It is common practice in Renesas BSP to specify the SiP memory
split by suffixing the DTB names with '-{2,4}x{2,4}g' [1].
Has this ever been discussed on ML?
Here in particular, it would allow M3-W+ 2x4GiB Salvator-XS and
M3-W+ 2x2GiB (or any other DRAM split flavor of) Salvator-XS to
coexist in harmony, if the latter pops up at any point.
[1] (rcar-3.9.6) ls -1 arch/arm64/boot/dts/renesas/*dtb | grep 'g.dtb'
arch/arm64/boot/dts/renesas/r8a7795-h3ulcb-4x2g.dtb
arch/arm64/boot/dts/renesas/r8a7795-salvator-xs-2x2g.dtb
arch/arm64/boot/dts/renesas/r8a7795-salvator-xs-4x2g.dtb
arch/arm64/boot/dts/renesas/r8a7796-salvator-xs-2x4g.dtb
--
Best Regards,
Eugeniu
Re: [RFC DO-NOT-MERGE PATCH] arm64: dts: renesas: R8A77961: Add Renesas M3-W+ (M3 ES3.0) SoC support
Hi Simon, On Sat, Aug 31, 2019 at 10:01:02AM +0200, Simon Horman wrote: [..] > +1 > > Your recollection of the decisions made around H3 and 4/5 digit identifiers > matches mine. And I agree that in hindsight we may have been better to use > a 5 digit identifier for H3 ES2.0. So I think it would be a good idea to > learn from this and use a 5 digit identifier for M3-W+. We can always > register unused identifiers if the need arises. > > For may the main drawback of this approach is that it is inconsistent > with the one we took for H3, which may lead to confusion. But I think > that we are better off to favour evolution over reuse in this case. > Or in other words, it seems like a good opportunity to learn from > our mistakes. Thank you for casting your thoughts. Much appreciated. -- Best Regards, Eugeniu
Re: [RFC DO-NOT-MERGE PATCH] arm64: dts: renesas: R8A77961: Add Renesas M3-W+ (M3 ES3.0) SoC support
Hi Geert, Many thanks for the recent clarifications. I got some M3 ES3.0 reference targets on my desk. So, if time permits, I might push some bring-up patches to you. -- Best Regards, Eugeniu
Re: [PATCH v2 1/3] usb: renesas_usbhs: enable DVSE interrupt
Hi Veeraiyan,
On Fri, Sep 06, 2019 at 02:03:49PM +0200, Veeraiyan Chidambaram wrote:
> From: Eugeniu Rosca
>
> Commit [1] enabled the possibility of checking the DVST (Device State
> Transition) bit of INTSTS0 (Interrupt Status Register 0) and calling
> the irq_dev_state() handler if the DVST bit is set. But neither
> commit [1] nor commit [2] actually enabled the DVSE (Device State
> Transition Interrupt Enable) bit in the INTENB0 (Interrupt Enable
> Register 0). As a consequence, irq_dev_state() handler is getting
> called as a side effect of other (non-DVSE) interrupts being fired,
> which definitely can't be relied upon, if DVST notifications are of
> any value.
>
> Why this doesn't hurt is because usbhsg_irq_dev_state() currently
> doesn't do much except of a dev_dbg(). Once more work is added to
> the handler (e.g. detecting device "Suspended" state and notifying
> other USB gadget components about it), enabling DVSE becomes a hard
> requirement. Do it in a standalone commit for better visibility and
> clear explanation.
>
> [1] f1407d5 ("usb: renesas_usbhs: Add Renesas USBHS common code")
> [2] 2f98382 ("usb: renesas_usbhs: Add Renesas USBHS Gadget")
>
> Signed-off-by: Eugeniu Rosca
> ---
> v2: No change
> v1: https://patchwork.kernel.org/patch/10581479/
It looks like this series changes the patch order of v1.
Could you kindly stick to the original order? Many thanks.
--
Best Regards,
Eugeniu.
Re: [PATCH v3] usb: gadget: udc: renesas_usb3: add suspend event support
Hi Shimoda-san,
On Fri, Sep 06, 2019 at 04:41:50AM +, Yoshihiro Shimoda wrote:
> Hello Eugeniu-san,
>
> > From: Eugeniu Rosca, Sent: Friday, September 6, 2019 4:07 AM
>
> >
> > I guess there are strong similarities between this patch and [3].
> > Would you like to pick [1-3], as they still apply cleanly to vanilla?
>
> Thank you for your comment! I completely forgot that you had submitted
> these [1-3] patches though, I'm thinking renesas_usbhs driver also should
> have this similar feature. I checked the [3] again and the commit log
> and the conditions should be fixed like this patch. Would you submit
> v2 patch series for renesas_usbhs driver? Or, May I submit it?
> Anything is OK to me.
Thank your for the prompt reply. It is very appreciated.
We'll make sure to submit the v2 of [1-3], as per your request.
>
> > [1] https://patchwork.kernel.org/patch/10581479/
> > ("[1/3] usb: renesas_usbhs: simplify usbhs_status_get_device_state()")
> > [2] https://patchwork.kernel.org/patch/10581485/
> > ("[2/3] usb: renesas_usbhs: enable DVSE interrupt")
> > [3] https://patchwork.kernel.org/patch/10581489/
> > ("usb: renesas_usbhs: add suspend event support in gadget mode")
--
Best Regards,
Eugeniu.
Re: [PATCH v3] usb: gadget: udc: renesas_usb3: add suspend event support
Hello Shimoda-san,
On Thu, Sep 05, 2019 at 11:07:02AM +, Yoshihiro Shimoda wrote:
> Hi Veeraiyan,
>
> > From: Veeraiyan Chidambaram, Sent: Thursday, September 5, 2019 6:18 PM
> >
> > In R-Car Gen3 USB 3.0 Function, if host is detached an interrupt
> > will be generated and Suspended state bit is set in interrupt status
> > register. Interrupt handler will call driver->suspend(composite_suspend)
> > if suspended state bit is set. composite_suspend will call
> > ffs_func_suspend which will post FUNCTIONFS_SUSPEND and will be consumed
> > by user space application via /dev/ep0.
> >
> > To be able to detect the host detach, USB_INT_1_B2_SPND to cover the
> > Suspended bit of the B2_SPND_OUT[9] from the USB Status Register
> > (USB_STA) register and perform appropriate action in the
> > usb3_irq_epc_int_1 function.
> >
> > Without this commit, disconnection of the phone from R-Car H3 ES2.0
> > Salvator-X CN11 port is not recognized and reverse role switch does
> > not happen. If phone is connected again it does not enumerate.
> >
> > With this commit, disconnection will be recognized and reverse role
> > switch will happen by a user space application. If phone is connected
> > again it will enumerate properly and will become visible in the
> > output of 'lsusb'.
> >
> > Signed-off-by: Veeraiyan Chidambaram
>
> Thank you for the patch!
>
> Reviewed-by: Yoshihiro Shimoda
>
> And, I tested this patch on my environment [1] and works correctly. So,
>
> Tested-by: Yoshihiro Shimoda
I guess there are strong similarities between this patch and [3].
Would you like to pick [1-3], as they still apply cleanly to vanilla?
[1] https://patchwork.kernel.org/patch/10581479/
("[1/3] usb: renesas_usbhs: simplify usbhs_status_get_device_state()")
[2] https://patchwork.kernel.org/patch/10581485/
("[2/3] usb: renesas_usbhs: enable DVSE interrupt")
[3] https://patchwork.kernel.org/patch/10581489/
("usb: renesas_usbhs: add suspend event support in gadget mode")
PS: Apologize for long silence in [3].
--
Best Regards,
Eugeniu.
Re: [RFC DO-NOT-MERGE PATCH] arm64: dts: renesas: R8A77961: Add Renesas M3-W+ (M3 ES3.0) SoC support
Hi Geert, Thanks for taking some time to reflect retrospectively on the bring-up experiences from the past and to summarize the lessons learned. On Fri, Aug 23, 2019 at 04:18:09PM +0200, Geert Uytterhoeven wrote: [..] > Actually R-Car H3 ES2.0 is r8a77951, while ES1.x is r8a77950. That's a great detail which I missed. I confirm below: - SoC HW manual "Rev.1.00 Apr 2018" maps R8A77951 to H3 ver2.0 - SoC HW manuals "Rev.1.50 Nov 2018" and "Rev.2.00 Jul 2019" map R8A77951 to H3 ver3.0 - Older Renesas docs refer to the earlier H3 ES1.x SoC as R8A77950 So, on the one hand, there _is_ a 'part number' update from H3 ver1 to rev2 and, on the other hand, there is no part number update from ver2 to ver3. My understanding of the latter (as a side note) is that there are no added/dropped HW features in ver3, hence the same 'part number'. [..] > When we started work on H3 ES2.0, it was considered an evolutionary step > from ES1.x, not a different SoC. We also were used to 4-digit IDs in > compatible values, as before the 5th digit was typically used to > indicate a minor difference, like a different package, or a different > ROM option. Hence we ignored the 5th digit, reused the compatible > values for H3 ES1.x, and went with soc_device_match() to differentiate, > where needed. Honestly, this still sounds sensible and intuitive, assuming the delta between the SoC models (differing in the 5th digit) is relatively low (not sure how to quantify it though and where the threshold is). One of the concerns related to soc_device_match() is that it sometimes doesn't play nicely with spinlocks, generating lockdep splats [1]. > However, it turned out H3 ES2.0 was more like a different SoC in the > same family: it has more similarities with R-Car M3-W ES1.0 than with > R-Car H3 ES1.x. > > In the mean time, with the advent of R-Car D3 and M3-N, > we also got used to 5 digits. Hence in hindsight, it might have been > better if we had considered H3 ES1.x and ES2.0 to be two different > SoCs. Thinking of the way H3-ES1.x support was added, now that H3-ES1.x is declared obsolete, we could probably reduce the image size by some tens of KiB (more?) by simply disabling CONFIG_ARCH_R8A77950 if its support was implemented as standalone CONFIG_ARCH? We now have to compile and link the H3-ES1.x functionality even if nobody needs it, which is a bit unfortunate. > > Given R-Car M3-W and M3-W+ may co-exist as separate SoCs, I think > approach B is the best approach, using separate DTS, compatible values, > Kconfig, and drivers, like we did for e.g. R-Car M3-N. > > What do you think? I still think B is the best option in terms of transparency (clear mapping between documentation and code), but I try to reconcile below recent concerns (any support appreciated): - After reviewing the "Engineering Change Notice for R8A77961", it seems to me that the number of differences between R8A77960 and R8A77961 is really low (much much lower than in the case of R8A77950->R8A77951 transition). Most (if not all) of the IP blocks present in R8A77960 and removed in R8A77961 [2] (i.e. fcpci0, fcpcs, ivdp1c, vdpb) are not even supported in vanilla. - I guess with this low amount of differences, R8A77961 will be an almost perfect twin of R8A77960. If so, then is the additional maintenance effort (resulting after bring-up) still justified? - Duplicating 'drivers/pinctrl/sh-pfc/pfc-r8a7796.c' (as it was done for M3-N, with 99% similarity in the contents) will increase the image size by roughly 50KiB [3]. Additional (albeit less significant) size increase is expected from the addition of other SoC-specific drivers. - [Minor] According to your feedback and to the best of our knowledge, both M3-W and M3-W+ are expected to reach the end user, which might create less motivation to really separate the two SoCs via CONFIG. What do you think about the above? Thanks! [1] lockdep splats generated by soc_device_match() https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ba164a49f8f739 https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=8037f4932ec5 https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=08cd9d10caff https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=34d3b527b70c https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=918f22c7b2ae https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=196d1399ffa8 https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=5ed4a312252e [2] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=8ba438fd03d5 [3] $ size drivers/pinctrl/sh-pfc/pfc-r8a7796*.o textdata bss dec hex filename 51660 0 0 51660c9cc drivers/pinctrl/sh-pfc/pfc-r8a77965.o 51628 0 0 51628c9ac drivers/pinctrl/sh-pfc/pfc-r8a7796.
[RFC DO-NOT-MERGE PATCH] arm64: dts: renesas: R8A77961: Add Renesas M3-W+ (M3 ES3.0) SoC support
Similar to the revision update from H3-ES1.x to H3-ES2.0, the update
from M3-ES1.x to M3-ES3.0, in addition to fixing HW bugs/erratas, drops
entire silicon IPs [1-2] (for cost efficiency and other reasons).
However, unlike in the H3 ES1.x->ES2.0 revision update, the M3-ES3.0
came with a new SoC id, i.e. r8a77961 (according to both [2] and
the updated SoC HW manual Rev.2.00 Jul 2019). The choice to allocate a
new identifier seems to strengthen the HW differences between M3-ES1.x
and M3-ES3.0 (as it is the case for M3N/r8a77965).
Given the above, there are several ways to differentiate between
M3-ES1.x and M3-ES3.0:
A. The BSP way [1]. Move/rename r8a7796.dtsi to r8a7796-es1.dtsi and
keep using r8a7796.dtsi for M3-ES3.x.
Pros:
* Resembles commit 291e0c4994d081 ("arm64: dts: r8a7795: Add support
for R-Car H3 ES2.0")
* Reuses the r8a7796 (e.g. sysc, cpg-mssr) drivers for r8a77961 (i.e.
minimizes the bring-up effort)
Cons:
* Deliberately diverges from the vendor documentation [2] by
ignoring the new SoC identifier r8a77961, i.e. leading to
inconsistencies in the names of the drivers and DTS
B. The approach taken in this patch, i.e. create a brand new
r8a77961.dtsi (similar to r8a77965.dtsi).
Pros:
* Reflects the reality documented by HW designers [2]
* Maintains drivers/DTS naming consistency and avoids mismatch between
documentation and code
Cons:
* higher bring-up effort than (A)
* more discussion is needed on whether it makes sense to separate:
- DTS only
- DTS + Kconfig (ARCH_R8A77961)
- DTS + Kconfig (ARCH_R8A77961) + drivers (sysc, cpg-mssr, other?)
Comments appreciated!
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77961.dtsi | 16
1 file changed, 16 insertions(+)
create mode 100644 arch/arm64/boot/dts/renesas/r8a77961.dtsi
diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
new file mode 100644
index ..7f784619be32
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the R-Car M3-W+ (R8A77961) ES3.x SoC
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include "r8a7796.dtsi"
+
+/*
+ * Here comes the delta between M3-W (M3 ES1.x) and M3-W+ (M3 ES3.0)
+ * described in:
+ * [1]
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=8ba438fd03d5
+ * ("arm64: dts: r8a7796: Add support for R-Car M3 ES3.0")
+ * [2] [Confidential] Engineering Change Notice for R8A77961
+ */
--
2.23.0
Re: [PATCH v2 2/5] arm64: dts: r8a7795: Add cpuidle support for CA53 cores
Hello Geert, hello Lorenzo, Many thanks for your comments and for the willingness to help. For your information, we've recently discovered that, with all the findings already described being absolutely valid for the reference targets, disabling CPUidle on the customer HW is apparently not enough to fix the audio dropouts. We will first try to identify those differences (both HW and SW) which keep the issue reproducible on the customer boards. Once this is hopefully understood, we'll come back with feedback. This investigation also happens to overlap with my vacation. Hence I plan to update you on this topic in 2-4 weeks from now. Thanks again. Best regards, Eugeniu.
Re: [PATCH v2 2/5] arm64: dts: r8a7795: Add cpuidle support for CA53 cores
On Fri, Jul 26, 2019 at 11:13:29AM +0200, Rosca, Eugeniu (ADITG/ESM1) wrote:
[..]
> The culprit BSP commits are:
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=3c3b44c752c4ee
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=902ff7caa32dc71c
>
> Further narrowing it down, it turns out the CA57 cpuidle support is
> not responsible for generating the issue. It's all about the CA53 idle
> enablement. The reference target is H3-ES2.0-Salvator-X (the problem
> originally emerged on M3-based customer HW).
[..]
Small amendment to the above (based on vanilla testing):
Version Issue reproduced?
(H3-ES2.0-Salvator-X)
v5.3-rc1-96-g6789f873ed37 No
v5.3-rc1-96-g6789f873ed37 + [1]No
v5.3-rc1-96-g6789f873ed37 + [2]No
v5.3-rc1-96-g6789f873ed37 + [1] + [2] Yes
[1] https://patchwork.kernel.org/patch/10769701/
("[v2,1/5] arm64: dts: r8a7795: Add cpuidle support for CA57 cores")
[2] https://patchwork.kernel.org/patch/10769689/
("[v2,2/5] arm64: dts: r8a7795: Add cpuidle support for CA53 cores")
--
Best Regards,
Eugeniu.
Re: [PATCH 0/2] serial: sh-sci: Fix .flush_buffer() issues
Hi Greg,
On Wed, Jul 03, 2019 at 07:30:50PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jun 28, 2019 at 03:02:00PM +0200, Eugeniu Rosca wrote:
[..]
> > I am doing this per-patch to allow patchwork to reflect the status of
> > each patch on the linux-renesas-soc front-page. AFAIK patchwork ignores
> > series-wide '*-by: Name ' signatures/tags.
>
> I don't use patchwork :)
How do you then collect all the "{Reviewed,Tested,etc}-by:" signatures
(each of which means sometimes hours of effort) in the hairy ML threads?
Patchwork makes it a matter of one click.
--
Best regards,
Eugeniu.
Re: [PATCH] dmaengine: rcar-dmac: Reject zero-length slave DMA requests
Hi Geert,
On Fri, Jun 28, 2019 at 02:10:01PM +0200, Geert Uytterhoeven wrote:
> On Wed, Jun 26, 2019 at 8:15 PM Eugeniu Rosca wrote:
[..]
> I'm not such a big fan of WARN()...
[..]
> > rcar-dmac e730.dma-controller: rcar_dmac_prep_slave_sg: bad parameter:
> > len=1, id=19
>
> Which would be followed by
>
> sh-sci e6e88000.serial: Failed preparing Tx DMA descriptor
>
> pointing to the sh-sci driver, right?
>
> The id=19 points to channel 0x13, i.e. SCIF2, according to
> arch/arm64/boot/dts/renesas/r8a7795.dtsi.
Thank you for the detailed rationale. Much appreciated.
FTR, the patch landed in vkoul/slave-dma.git, as commit
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/commit/?h=next&id=78efb76ab4dfb8f
("dmaengine: rcar-dmac: Reject zero-length slave DMA requests")
--
Best Regards,
Eugeniu.
Re: [PATCH 2/2] serial: sh-sci: Terminate TX DMA during buffer flushing
On Mon, Jun 24, 2019 at 02:35:40PM +0200, Geert Uytterhoeven wrote:
[..]
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index d4504daff99263f5..d18c680aa64b3427 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1656,11 +1656,18 @@ static void sci_free_dma(struct uart_port *port)
>
> static void sci_flush_buffer(struct uart_port *port)
> {
> + struct sci_port *s = to_sci_port(port);
> +
> /*
>* In uart_flush_buffer(), the xmit circular buffer has just been
> - * cleared, so we have to reset tx_dma_len accordingly.
> + * cleared, so we have to reset tx_dma_len accordingly, and stop any
> + * pending transfers
>*/
> - to_sci_port(port)->tx_dma_len = 0;
> + s->tx_dma_len = 0;
> + if (s->chan_tx) {
> + dmaengine_terminate_async(s->chan_tx);
> + s->cookie_tx = -EINVAL;
> + }
I am seeing a similar solution being employed by other tty/serial
drivers [1]. One major difference is that those drivers make use of
dmaengine_terminate_all(). Since the latter is deprecated starting
with commit [2], the API choice looks reasonable to me.
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
[1] git grep -W "static void.*flush_buffer" -- drivers/tty/serial/ | grep
dmaengine_terminate
drivers/tty/serial/fsl_lpuart.c-
dmaengine_terminate_all(sport->dma_tx_chan);
drivers/tty/serial/imx.c- dmaengine_terminate_all(sport->dma_chan_tx);
drivers/tty/serial/serial-tegra.c-
dmaengine_terminate_all(tup->tx_dma_chan);
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b36f09c3c441a6
("dmaengine: Add transfer termination synchronization support")
--
Best Regards,
Eugeniu.
Re: [PATCH 0/2] serial: sh-sci: Fix .flush_buffer() issues
Hi Wolfram, On Fri, Jun 28, 2019 at 02:55:34PM +0200, Wolfram Sang wrote: [..] > If you could formally add such a tag: > > Tested-by: > > (maybe also Acked-by: or Reviewed-by:, dunno if you think it is > apropriate) > > to the patches, this would be much appreciated and will usually speed up > the patches getting applied. > > Thanks for your help! I am doing this per-patch to allow patchwork to reflect the status of each patch on the linux-renesas-soc front-page. AFAIK patchwork ignores series-wide '*-by: Name ' signatures/tags. -- Best Regards, Eugeniu.
Re: [PATCH 1/2] serial: sh-sci: Fix TX DMA buffer flushing and workqueue races
Hi Geert,
One bikeshedding below.
On Mon, Jun 24, 2019 at 02:35:39PM +0200, Geert Uytterhoeven wrote:
[..]
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index abc705716aa094fd..d4504daff99263f5 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1398,6 +1398,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
> struct circ_buf *xmit = &port->state->xmit;
> unsigned long flags;
> dma_addr_t buf;
> + int head, tail;
>
> /*
>* DMA is idle now.
> @@ -1407,16 +1408,23 @@ static void sci_dma_tx_work_fn(struct work_struct
> *work)
>* consistent xmit buffer state.
>*/
> spin_lock_irq(&port->lock);
> - buf = s->tx_dma_addr + (xmit->tail & (UART_XMIT_SIZE - 1));
> + head = xmit->head;
> + tail = xmit->tail;
> + buf = s->tx_dma_addr + (tail & (UART_XMIT_SIZE - 1));
> s->tx_dma_len = min_t(unsigned int,
> - CIRC_CNT(xmit->head, xmit->tail, UART_XMIT_SIZE),
> - CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE));
> - spin_unlock_irq(&port->lock);
> + CIRC_CNT(head, tail, UART_XMIT_SIZE),
> + CIRC_CNT_TO_END(head, tail, UART_XMIT_SIZE));
> + if (!s->tx_dma_len) {
> + /* Transmit buffer has been flushed */
> + spin_unlock_irq(&port->lock);
> + return;
Since we can now return before using the result stored in 'buf', we
could relocate the 'buf' calculation next to the return statement, just
before calling dmaengine_prep_slave_single(), as micro-optimization.
> + }
>
> desc = dmaengine_prep_slave_single(chan, buf, s->tx_dma_len,
> DMA_MEM_TO_DEV,
> DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
Otherwise:
Reviewed-by: Eugeniu Rosca
Tested-by: Eugeniu Rosca
--
Best Regards,
Eugeniu.
Re: [PATCH 0/2] serial: sh-sci: Fix .flush_buffer() issues
Hi Geert, On Fri, Jun 28, 2019 at 01:51:25PM +0200, Geert Uytterhoeven wrote: [..] > If a serial port is used as a console, the port is used for both DMA > (normal use) and PIO (serial console output). The latter can have a > negative impact on the former, aggravating existing bugs, or triggering > more races, even in the hardware. So I think it's better to be more > cautious and keep DMA disabled for the console. Thanks for the extensive and comprehensible replies. No more questions from my end. Looking forward to picking the patches from vanilla/stable trees. -- Best Regards, Eugeniu.
Re: [Announce] Renesas SoC Co-Maintainer
Hello Simon, Thank you for the amazing support and responsiveness delivered to the R-Car users and beyond. "horms" is like an emblem you cannot take away from the Renesas kernel. It is imprinted in the names of repositories and inspires quality, trust and consistency. Greatest wishes on behalf of all those whose experience was enriched by your contributions and maintainership. -- Best Regards, Eugeniu.
Re: [PATCH] dmaengine: rcar-dmac: Reject zero-length slave DMA requests
Hi All,
On Mon, Jun 24, 2019 at 02:38:18PM +0200, Geert Uytterhoeven wrote:
[..]
> - if (rchan->mid_rid < 0 || !sg_len) {
> + if (rchan->mid_rid < 0 || !sg_len || !sg_dma_len(sgl)) {
> dev_warn(chan->device->dev,
>"%s: bad parameter: len=%d, id=%d\n",
>__func__, sg_len, rchan->mid_rid);
Just wanted to share the WARN output proposed by Wolfram in
https://patchwork.kernel.org/patch/11012991/#22721733
in case the issue discussed in [1] is reproduced with this patch:
[2.065337] [ cut here ]
[2.065346] rcar_dmac_prep_slave_sg:
[2.065394] WARNING: CPU: 2 PID: 252 at drivers/dma/sh/rcar-dmac.c:1169
rcar_dmac_prep_slave_sg+0x50/0xc4
[2.065397] Modules linked in:
[2.065407] CPU: 2 PID: 252 Comm: kworker/2:1 Not tainted
5.2.0-rc6-00016-g2bfb85ba1481-dirty #26
[2.065410] Hardware name: Renesas H3ULCB Kingfisher board based on r8a7795
ES2.0+ (DT)
[2.065420] Workqueue: events sci_dma_tx_work_fn
[2.065425] pstate: 4005 (nZcv daif -PAN -UAO)
[2.065430] pc : rcar_dmac_prep_slave_sg+0x50/0xc4
[2.065434] lr : rcar_dmac_prep_slave_sg+0x50/0xc4
[2.065436] sp : 112ebd00
[2.065438] x29: 112ebd00 x28:
[2.065443] x27: 8006fa367138 x26: 10c5bce8
[2.065447] x25: 000738b1d000 x24:
[2.065451] x23: 10b76e00 x22: 10a18000
[2.065455] x21: 0001 x20: 8006f9b5a080
[2.065459] x19: 107adc86 x18:
[2.065462] x17: x16:
[2.065466] x15: x14:
[2.065469] x13: 0004 x12: 10a35000
[2.065473] x11: 10b12981 x10: 0040
[2.065477] x9 : 013e x8 : 10b1b73b
[2.065481] x7 : x6 : 0001
[2.065484] x5 : 8006ff72f7c0 x4 : 0001
[2.065488] x3 : 0007 x2 : 0007
[2.065491] x1 : 878c73041cedc400 x0 :
[2.065495] Call trace:
[2.065500] rcar_dmac_prep_slave_sg+0x50/0xc4
[2.065504] sci_dma_tx_work_fn+0xd8/0x1d4
[2.065511] process_one_work+0x1dc/0x394
[2.065515] worker_thread+0x21c/0x308
[2.065520] kthread+0x118/0x128
[2.065527] ret_from_fork+0x10/0x18
[2.065530] ---[ end trace 75fc17d9000f1224 ]---
At first glance, it seems to give more details compared to:
rcar-dmac e730.dma-controller: rcar_dmac_prep_slave_sg: bad parameter:
len=1, id=19
[1] https://patchwork.kernel.org/cover/11012981/
--
Best regards,
Eugeniu.
Re: [PATCH 0/2] serial: sh-sci: Fix .flush_buffer() issues
Hi Geert, CC: George
On Mon, Jun 24, 2019 at 02:35:38PM +0200, Geert Uytterhoeven wrote:
> Hi Greg, Jiri,
>
> This patch series attempts to fix the issues Eugeniu Rosca reported
> seeing, where .flush_buffer() interfered with transmit DMA operation[*].
>
> There's a third patch "dmaengine: rcar-dmac: Reject zero-length slave
> DMA requests", which is related to the issue, but further independent,
> hence submitted separately.
>
> Eugeniu: does this fix the issues you were seeing?
Many thanks for both sh-sci and the rcar-dmac patches.
The fixes are very much appreciated.
> Geert Uytterhoeven (2):
> serial: sh-sci: Fix TX DMA buffer flushing and workqueue races
> serial: sh-sci: Terminate TX DMA during buffer flushing
>
> drivers/tty/serial/sh-sci.c | 33 -
> 1 file changed, 24 insertions(+), 9 deletions(-)
I reserved some time to get a feeling about how the patches behave on
a real system (H3-ES2.0-ULCB-KF-M06), so here come my observations.
First of all, the issue I have originally reported in [0] is only
reproducible in absence of [4]. So, one of my questions would be how
do you yourself see the relationship between [1-3] and [4]?
That said, all my testing assumes:
- Vanilla tip v5.2-rc6-15-g249155c20f9b with [4] reverted.
- DEBUG is undefined in {sh-sci.c,rcar-dmac.c}, since I've noticed
new issues arising in the debug build, which are unrelated to [0].
Below is the summary of my findings:
Version IS [0] Is console Error message when
(vanilla+X)reproduced? usable after [0] [0] is reproduced
is reproduced?
-[4] Yes No[5]
-[4]+[1] Yes No-
-[4]+[2] Yes Yes [5]
-[4]+[3] Yes Yes [6]
-[4]+[1]+[2] No- -
-[4]+[1]+[2]+[3] No- -
pure vanilla No- -
This looks a little too verbose, but I thought it might be interesting.
The story which I see is that [1] does not fix [0] alone, but it seems
to depend on [2]. Furthermore, if cherry picked alone, [1] makes the
matters somewhat worse in the sense that it hides the error [5].
My only question is whether [1-3] are supposed to replace [4] or they
are supposed to happily coexist. Since I don't see [0] being reproduced
with [1-3], I personally prefer to re-enable DMA on SCIF (when the
latter is used as console) so that more features and code paths are
exercised to increase test coverage.
[0] https://lore.kernel.org/lkml/20190504004258.23574-3-ero...@de.adit-jv.com/
[1] https://patchwork.kernel.org/patch/11012983/
("serial: sh-sci: Fix TX DMA buffer flushing and workqueue races")
[2] https://patchwork.kernel.org/patch/11012987/
("serial: sh-sci: Terminate TX DMA during buffer flushing")
[3] https://patchwork.kernel.org/patch/11012991/
("dmaengine: rcar-dmac: Reject zero-length slave DMA requests")
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=099506cbbc79c0
("serial: sh-sci: disable DMA for uart_console")
[5] rcar-dmac e730.dma-controller: Channel Address Error
[6] rcar-dmac e730.dma-controller: rcar_dmac_prep_slave_sg: bad parameter:
len=1, id=19
sh-sci e6e88000.serial: Failed preparing Tx DMA descriptor
Thanks!
--
Best regards,
Eugeniu.
Re: [PATCH] wlcore/wl18xx: Add invert-irq OF property for physically inverted IRQ
Hi,
cc: Linus Walleij
On Tue, Jun 11, 2019 at 10:00:41AM +0100, Marc Zyngier wrote:
> On 11/06/2019 09:45, Geert Uytterhoeven wrote:
> > CC irqchip
> >
> > Original thread at
> > https://lore.kernel.org/lkml/20190607172958.20745-1-ero...@de.adit-jv.com/
> >
> > On Mon, Jun 10, 2019 at 10:30 AM Tony Lindgren wrote:
> >> * Kalle Valo [190610 07:01]:
> >>> Eugeniu Rosca writes:
> >>>
> >>>> The wl1837mod datasheet [1] says about the WL_IRQ pin:
> >>>>
> >>>> ---8<---
> >>>> SDIO available, interrupt out. Active high. [..]
> >>>> Set to rising edge (active high) on powerup.
> >>>> ---8<---
> >>>>
> >>>> That's the reason of seeing the interrupt configured as:
> >>>> - IRQ_TYPE_EDGE_RISING on HiKey 960/970
> >>>> - IRQ_TYPE_LEVEL_HIGH on a number of i.MX6 platforms
> >>>>
> >>>> We assert that all those platforms have the WL_IRQ pin connected
> >>>> to the SoC _directly_ (confirmed on HiKey 970 [2]).
> >>>>
> >>>> That's not the case for R-Car Kingfisher extension target, which carries
> >>>> a WL1837MODGIMOCT IC. There is an SN74LV1T04DBVR inverter present
> >>>> between the WLAN_IRQ pin of the WL18* chip and the SoC, effectively
> >>>> reversing the requirement quoted from [1]. IOW, in Kingfisher DTS
> >>>> configuration we would need to use IRQ_TYPE_EDGE_FALLING or
> >>>> IRQ_TYPE_LEVEL_LOW.
> >>>>
> >>>> Unfortunately, v4.2-rc1 commit bd763482c82ea2 ("wl18xx: wlan_irq:
> >>>> support platform dependent interrupt types") made a special case out
> >>>> of these interrupt types. After this commit, it is impossible to provide
> >>>> an IRQ configuration via DTS which would describe an inverter present
> >>>> between the WL18* chip and the SoC, generating the need for workarounds
> >>>> like [3].
> >>>>
> >>>> Create a boolean OF property, called "invert-irq" to specify that
> >>>> the WLAN_IRQ pin of WL18* is connected to the SoC via an inverter.
> >>>>
> >>>> This solution has been successfully tested on R-Car H3ULCB-KF-M06 using
> >>>> the DTS configuration [4] combined with the "invert-irq" property.
> >>>>
> >>>> [1] http://www.ti.com/lit/ds/symlink/wl1837mod.pdf
> >>>> [2]
> >>>> https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/
> >>>> [3]
> >>>> https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WL-side.patch
> >>>> [4] https://patchwork.kernel.org/patch/10895879/
> >>>> ("arm64: dts: ulcb-kf: Add support for TI WL1837")
> >>>>
> >>>> Signed-off-by: Eugeniu Rosca
> >>>
> >>> Tony&Eyal, do you agree with this?
> >>
> >> Yeah if there's some hardware between the WLAN device and the SoC
> >> inverting the interrupt, I don't think we have clear a way to deal
> >> with it short of setting up a separate irqchip that does the
> >> translation.
> >
> > Yeah, inverting the interrupt type in DT works only for simple devices,
> > that don't need configuration.
> > A simple irqchip driver that just inverts the type sounds like a good
> > solution to me. Does something like that already exists?
>
> We already have plenty of that in the tree, the canonical example
> probably being drivers/irqchip/irq-mtk-sysirq.c. It should be pretty
> easy to turn this driver into something more generic.
I don't think drivers/irqchip/irq-mtk-sysirq.c can serve the
use-case/purpose of this patch. The MTK driver seems to be dealing with
the polarity inversion of on-SoC interrupts which are routed to GiC,
whereas in this patch we are talking about an off-chip interrupt
wired to R-Car GPIO controller.
It looks to me that the nice DTS sketch shared by Linus Walleij in [5]
might come closer to the concept proposed by Geert? FWIW, the
infrastructure/implementation to make this possible is still not ready.
One question to the wlcore/wl18xx maintainers: Why exactly do you give
freedom to users to set the interrupt as LEVEL_LOW/EDGE_FALLING [6]?
Apparently, this:
- complicates the wl18xx driver, thus increasing the chance for bugs
- is not supposed to reflect any HW differences between board
Re: [PATCH] serial: sh-sci: disable DMA for uart_console
Hi George,
I am able to reproduce the SCIF2 console freeze described in the
referenced patchwork link using M3-ES1.1-Salvator-XS and recent
v5.1-9573-gb970afcfcabd kernel.
I confirm the behavior is healed with this patch. Thanks!
Hope to see it accepted soon, since it fixes a super annoying
console breakage every fourth boot or so on lots of R-Car3 targets.
Tested-by: Eugeniu Rosca
On Thu, May 09, 2019 at 10:43:30AM -0400, George G. Davis wrote:
> As noted in commit 84b40e3b57ee ("serial: 8250: omap: Disable DMA for
> console UART"), UART console lines use low-level PIO only access functions
> which will conflict with use of the line when DMA is enabled, e.g. when
> the console line is also used for systemd messages. So disable DMA
> support for UART console lines.
>
> Fixes: https://patchwork.kernel.org/patch/10929511/
> Reported-by: Michael Rodin
> Cc: Eugeniu Rosca
> Signed-off-by: George G. Davis
> ---
> drivers/tty/serial/sh-sci.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 3cd139752d3f..885b56b1d4e4 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1557,6 +1557,9 @@ static void sci_request_dma(struct uart_port *port)
>
> dev_dbg(port->dev, "%s: port %d\n", __func__, port->line);
>
> + if (uart_console(port))
> + return; /* Cannot use DMA on console */
> +
> if (!port->dev->of_node)
> return;
>
> --
> 2.7.4
>
--
Best Regards,
Eugeniu.
Re: Automated/remote flashing of R-Car3
Hi Marek, On Tue, May 07, 2019 at 06:09:10PM +0200, Marek Vasut wrote: [..] > >>> 1.c Use OpenOCD > >>> + Presumably same advantages as using a Lauterbach > >>> + Based on Kieran's https://github.com/kbingham/renesas-jtag > >>> and on Adam's https://github.com/ntfreak/openocd/commit/1afec4f561392 > >>> the solution is currently in use. > >>> ? Any ideas on the model/price of the JTAG adapter? > >> > >> Any FT2232H (the H is important, due to MPSSE) works. > >> I like Flyswatter2 from TinCanTools. > >> > >>> ? Not tested. Any patches needed on top of vanilla OpenOCD? > >> > >> http://openocd.zylin.com/5149 and related ones, it adds RPC HF support. > >> However, there are two problems with this: > >> 1) Even with buffered write, the programming is slow > >>- This could be improved by running code on one of the Gen3 CPUs > >> instead of whacking registers via JTAG adapter. I believe that's > >> what lauterbach and everyone else does too. The data upload to > >> SRAM/DRAM is fast via JTAG, register IO is not great. > >> 2) LifeC locks the RPC HF access > >>- This is a problem, since the JTAG probe cannot access it once > >> it's locked. There might be a way around it, but it's rather > >> nasty -- use boundary scan test mode to either flip MD pins or > >> access the HF bus directly and bitbang at least erase command > >> to wipe the first few sectors, then reset the CPU and have it > >> drop to SCIF loader mode, then stop the CPU and reprogram the > >> HF (since the SCIF loader runs in EL3 and does not touch the > >> lifec settings. > >> > >> Neither of 1) and 2) is implemented, but can be implemented if there is > >> interest. > > > > 1) looks like a performance issue to me (suboptimal flashing time). > > To be honest, I don't think this is a deal-breaker, assuming that > > erasing/re-writing the whole 64MiB HF doesn't exceed ~10-15min. > > It is also my understanding this is subject of future optimization. > > It will have to be optimized further. > > > 2) looks like a functional issue (insufficient permission to > > write-access HF). To make things clear, could you please stress if > > http://openocd.zylin.com/5149 already allows updating ATF/U-Boot/OPTEE > > on HF of R-Car Gen3 or is it still awaiting some fixes? > > You can read/write/erase the HF with it. Just keep in mind the HF has to > be unlocked. > > Maybe there is some magic/sectret way to unlock the LifeC RPC access > restriction via JTAG, but we don't know about it. Well, "unlocking" HF via LifeC registers (set in BL2) is pretty much equivalent to diverging from the Renesas-delivered BL2/IPL, by accepting a solution like [0] referenced in my initial e-mail (linked again below). Spawning multiple build variants of ATF (i.e. tested vs released) is what we would like to avoid. Thanks for pointing out this limitation. Hopefully it can be addressed in a future iteration of http://openocd.zylin.com/5149. [0] https://github.com/CogentEmbedded/meta-rcar/blob/v3.15.0/meta-rcar-gen3-adas/recipes-bsp/arm-trusted-firmware/files/0001-plat-renesas-rcar-Make-RPC-secure-settings-optional.patch -- Best Regards, Eugeniu.
Re: Automated/remote flashing of R-Car3
On Tue, May 07, 2019 at 06:09:10PM +0200, Marek Vasut wrote: [..] > >>> 1.d. Use CPLD Configurator > >>> + H3_M3_StarterKit_Configurator.exe is a Windows tool shipped by > >>> Renesas, hence readily available, which allows to modify the MD > >>> pins, to conveniently switch between QSPI/Hyperflash/SCIF > >>> boot mode from a GUI > >>> + Most of the advantages pointed out above > >>> - ULCB-only solution (i.e. does not apply to Salvator-X) > >>> - Requires a Windows host > >> > >> Where can I obtain this and are there sources / documentation available? > > > > I am able to find below related package freely available: > > https://elinux.org/File:H3_StarterKit_CPLD_Update_20190408.zip > > > > Unfortunately, it doesn't include H3_M3_StarterKit_Configurator.exe. > > The user who uploaded the file is https://elinux.org/User:RenesasJa. > > Are you aware of any messaging/commenting feature on elinux.org? > > If not, I hope Michael (CC-ed) can answer your question. Hopefully > > he sees this message. If not, I can forward your question to him via > > mantis. > > It would be also interesting to obtain the CPLD sources and be able to > synthesise custom CPLD bitstreams for automated testing. Is my understanding correct that these "CPLD bitstreams" (once known) could be implemented in U-Boot's board/renesas/ulcb/cpld.c? -- Best Regards, Eugeniu.
Re: Automated/remote flashing of R-Car3
Hi Marek, Thanks for the swift reply and for the useful references/links. On Tue, May 07, 2019 at 03:23:12PM +0200, Marek Vasut wrote: > On 5/7/19 12:41 PM, Eugeniu Rosca wrote: > > Dear Marek, dear Kieran, > > Hi, > > [...] > > > 1.c Use OpenOCD > > + Presumably same advantages as using a Lauterbach > > + Based on Kieran's https://github.com/kbingham/renesas-jtag > > and on Adam's https://github.com/ntfreak/openocd/commit/1afec4f561392 > > the solution is currently in use. > > ? Any ideas on the model/price of the JTAG adapter? > > Any FT2232H (the H is important, due to MPSSE) works. > I like Flyswatter2 from TinCanTools. > > > ? Not tested. Any patches needed on top of vanilla OpenOCD? > > http://openocd.zylin.com/5149 and related ones, it adds RPC HF support. > However, there are two problems with this: > 1) Even with buffered write, the programming is slow >- This could be improved by running code on one of the Gen3 CPUs > instead of whacking registers via JTAG adapter. I believe that's > what lauterbach and everyone else does too. The data upload to > SRAM/DRAM is fast via JTAG, register IO is not great. > 2) LifeC locks the RPC HF access >- This is a problem, since the JTAG probe cannot access it once > it's locked. There might be a way around it, but it's rather > nasty -- use boundary scan test mode to either flip MD pins or > access the HF bus directly and bitbang at least erase command > to wipe the first few sectors, then reset the CPU and have it > drop to SCIF loader mode, then stop the CPU and reprogram the > HF (since the SCIF loader runs in EL3 and does not touch the > lifec settings. > > Neither of 1) and 2) is implemented, but can be implemented if there is > interest. 1) looks like a performance issue to me (suboptimal flashing time). To be honest, I don't think this is a deal-breaker, assuming that erasing/re-writing the whole 64MiB HF doesn't exceed ~10-15min. It is also my understanding this is subject of future optimization. 2) looks like a functional issue (insufficient permission to write-access HF). To make things clear, could you please stress if http://openocd.zylin.com/5149 already allows updating ATF/U-Boot/OPTEE on HF of R-Car Gen3 or is it still awaiting some fixes? > > > 1.d. Use CPLD Configurator > > + H3_M3_StarterKit_Configurator.exe is a Windows tool shipped by > > Renesas, hence readily available, which allows to modify the MD > > pins, to conveniently switch between QSPI/Hyperflash/SCIF > > boot mode from a GUI > > + Most of the advantages pointed out above > > - ULCB-only solution (i.e. does not apply to Salvator-X) > > - Requires a Windows host > > Where can I obtain this and are there sources / documentation available? I am able to find below related package freely available: https://elinux.org/File:H3_StarterKit_CPLD_Update_20190408.zip Unfortunately, it doesn't include H3_M3_StarterKit_Configurator.exe. The user who uploaded the file is https://elinux.org/User:RenesasJa. Are you aware of any messaging/commenting feature on elinux.org? If not, I hope Michael (CC-ed) can answer your question. Hopefully he sees this message. If not, I can forward your question to him via mantis. Thank you! > > -- > Best regards, > Marek Vasut -- Best Regards, Eugeniu.
Automated/remote flashing of R-Car3
Dear Marek, dear Kieran, CC: Michael, Gotthard, Adam CC: U-Boot, linux-renesas-soc CC: ADIT We in ADIT have recently set up some remote servers for CI deployment/testing of R-Car3 targets (Salvator-X and ULCB-KF). Since re-flashing of "firmware" (i.e. ATF, U-Boot and OPTEE-OS) on Hyperflash involves manipulation of on-board DIP switches and direct access to the targets is difficult/infeasible in our case, we've started to look for a solution of flashing the boards remotely. Anticipating that you might have potentially implemented this in your private setups, would you please provide your valuable feedback on below thoughts which we have collected together with Michael Dege? 1.a Use Lauterbach + No changes in Renesas boot concept/flow + No changes in release/production ATF build flags + No physical changes on the boards + Immune to corrupted/wrong binaries + Tested. Fast and reliable. - Keeping one lauterbach/target for flashing purpose only _is_ expensive 1.b Replace relevant on-board DIP switches with remote-controlled relays + No changes in Renesas boot concept/flow + No changes in release/production ATF build flags + If set up properly, presumably immune to corrupted/wrong binaries - Major/intrusive physical changes required on the board - Lost warranty for the modified targets 1.c Use OpenOCD + Presumably same advantages as using a Lauterbach + Based on Kieran's https://github.com/kbingham/renesas-jtag and on Adam's https://github.com/ntfreak/openocd/commit/1afec4f561392 the solution is currently in use. ? Any ideas on the model/price of the JTAG adapter? ? Not tested. Any patches needed on top of vanilla OpenOCD? 1.d. Use CPLD Configurator + H3_M3_StarterKit_Configurator.exe is a Windows tool shipped by Renesas, hence readily available, which allows to modify the MD pins, to conveniently switch between QSPI/Hyperflash/SCIF boot mode from a GUI + Most of the advantages pointed out above - ULCB-only solution (i.e. does not apply to Salvator-X) - Requires a Windows host 2.a Enable non-secure access to Hyperflash and write from U-Boot/Linux + Implemented/used by CogentEmbedded via [0-3] + No changes in Renesas boot concept/flow + No physical changes on the boards + No additional HW adapters - Requires changes in ATF build flags, i.e. the tested build flavor of ATF diverges from what's used in production - Not immune to corrupted/wrong binaries and failures/interruptions, i.e. flashing a wrong binary would result in unbootable target (afterwards, direct access to the target is needed for either MiniMonitor or Lauterbach-based flashing) 2.b Implement a TA to get write access to Hyperflash via OPTEE OS + Same pros as (2.a) plus no need to build a special ATF variant - Same as (2.a), not immune to flashing failures - Not yet developed/tested 3 Add flash writer [4] to the boot chain between BL1 and BL2 + No physical changes on the boards + No additional HW adapters + Presumably no changes in release/production ATF build flags + Presumably immune to flashing failures, since it is assumed to be executed before BL2 - Changes/mangles the Renesas boot concept/flow - Not yet developed/tested. As pointed out by Michael offline, the implementation might be not so trivial - Expensive (initial development + new bugs due to diverging from Renesas) 4 Store/load (BL31, cert_header, U-Boot and OPTEE) to/from eMMC + This is a PoC/dirty solution received in the Renesas Android release, which relies on the fact that eMMC can be written to regardless of DIP switch settings. Writing to eMMC can be done via fastboot. + No physical changes on the boards + No additional HW adapters - BL2 remains in Hyperflash, hence can't be updated with this approach - Not immune to flashing failures - Not supported in vanilla or in Renesas Yocto ATF It's really amazing that with this great number of options, there is no perfect solution. If you have any comments, especially in the area of OpenOCD, those would be highly appreciated. TIA! [0] https://github.com/CogentEmbedded/meta-rcar/blob/v3.15.0/meta-rcar-gen3-adas/recipes-bsp/arm-trusted-firmware/files/0001-plat-renesas-rcar-Make-RPC-secure-settings-optional.patch [1] https://github.com/CogentEmbedded/meta-rcar/blob/v3.15.0/meta-rcar-gen3-adas/recipes-bsp/u-boot/u-boot/0040-arm-renesas-Enable-RPC-HF-MTD-support-for-all-Salvat.patch [2] https://github.com/CogentEmbedded/meta-rcar/blob/v3.15.0/meta-rcar-gen3-adas/recipes-bsp/u-boot/u-boot/0041-arm-renesas-Enable-RPC-HF-MTD-support-for-all-ULCB-b.patch [3] https://github.com/CogentEmbedded/meta-rcar/blob/v3.15.0/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0012-mtd-Add-RPC-HyperFlash-driver.patch [4] https://github.com/renesas-rcar/flash_writer -- Best Regards, Eugeniu.
Re: [PATCH 2/6] Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"
Hi Geert,
On Mon, May 06, 2019 at 12:02:41PM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
>
> Thanks for your report!
Thanks for your feedback.
>
> On Sat, May 4, 2019 at 2:45 AM Eugeniu Rosca wrote:
> > This reverts commit 97f26702bc95b5c3a72671d5c6675e4d6ee0a2f4.
> >
> > Here is the story behind this revert.
> >
> > Mainline commit [0] landed in the stable tree as commit [1], from where
> > it reached us in the form of regular stable update. After that, Michael
> > started to report occasional (30-50%) freezes of serial console on
> > booting M3-ES1.1-Salvator-XS. Same happened on M3-ES1.1-Salvator-X.
> >
> > Every time the issue occurs, the serial console outputs below [2]
> > before becoming totally unresponsive and printing nothing else:
> > rcar-dmac e730.dma-controller: Channel Address Error
> >
> > Git bisecting shows that the problem is contributed by commits [0-1].
> >
> > While we can't be 100% certain (since we don't have the SCIF design docs
> > revealing its internal implementation detail) we think there is plenty
> > of evidence to assume that DMA is not supported on SCIF2, hence should
> > stay disabled on this specific channel:
> >
> > - Excerpt from Chapter 17. Direct Memory Access Controller for System
> >(SYS-DMAC) of R19UH0105EJ0150 Rev.1.50:
> >-8<-
> >[H3, H3-N, M3-W, V3M, V3H, D3, M3-N, E3]
> >The following modules can issue on-chip peripheral module requests.
> >[..] HSCIF0/1/2/3/4, [..] SCIF0/1/3/4/5,
> >-8<-
> >
> > - Excerpt from RENESAS_RCH3M3M3NE3_SCIF_UME_v2.00.pdf (Yocto v3.15.0):
> >-8<-
> >DMA Transfer:
> >- Support: SCIF0, SCIF1, SCIF3, SCIF4, SCIF5
> >- Not support: SCIF2
> >-8<-
>
> > - Disabled SCIF2 DMA in official Renesas v4.9/v4.14 kernels, e.g. see:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=e79c418fda8c
>
> Table 17.5 ("Selecting On-Chip Peripheral Module Request Modes") of
> "R-Car Series, 3rd Generation User’s Manual: Hardware" gained entries
> for SCIF2 in Revision 1.50 of the document, but it seems 17.1.1
> ("Features") and Table 17.6 ("Data Length of DMA Transfer for Each of
> the On-Chip Peripheral Modules") were forgotten to be updated.
> The addition of the entry for SCIF2 is also mentioned in "Renesas
> Technical Update TN-RCT-S019A/E / R-Car M3-W Additional Explanation for
> Direct Memory Access Controller for System (SYS-DMAC)".
> Unfortunately both documents report wrong MID/RID values, due to a
> hexadecimal vs. decimal mistake, which were corrected in the Feb 12
> errata for Rev. 1.50.
I do observe now that the most recent Rev. 1.50 of "R-Car Series, 3rd
Generation User’s Manual: Hardware" does update _some_ of its internal
chapters/tables to reflect the support of DMA on SCIF2. These SCIF2
changes look to be also tracked in the "Revision History" companion doc:
Rev | Date | Page | Summary
1.50 | Nov 30, 2018 | 17-86-87 | Table 17.5 Selecting On-Chip Peripheral
Module Request Modes: DMA Transfer
Request Source, changed. SCIF2
reception and SCIF2 transmission, added
| 17-91| Table 17.6 Data Length of DMA Transfer
for Each of the On-Chip Peripheral
Modules: SCIF2, added
As you have already stated, it looks like certain chapters like
"17.1.1 Features" didn't receive a proper update, generating confusion.
I will report this in parallel to Renesas Duesseldorf.
>
> So in my understanding, and according to my testing, DMA has always
> worked for SCIF2 on (at least) R-Car H3 ES1.0/2.0, M3-W, and M3-N.
Well, my testing shows different results. Using M3-W-ES1.1-Salvator-XS,
I can reproduce the issue since v4.17 (also reproduced on v4.18, v4.19
and v5.1 with cherry picking 97f26702bc95b5 ("arm64: dts: renesas:
r8a7796: Enable DMA for SCIF2") where appropriate).
> However, early firmware versions (before IPL and Secure Monitor
> Rev1.0.6, released on Feb 25, 2016) prohibited the use of SYS-DMAC2,
> cfr. commit eb21089c32054ecd ("arm64: dts: renesas: r8a7795: Add missing
> SYS-DMAC2 dmas").
I use a very recent Rev2.0.2 of
https://github.com/renesas-rcar/arm-trusted-firmware .
>
> Perhaps some firmware versions may impose additional restrictions?
I would have some suspicions about ATF if the issue was consistent.
Since it is not, I believe there is a
Re: [PATCH 1/6] serial: sh-sci: Reveal ptrval in dev_dbg
On Mon, May 06, 2019 at 06:46:57PM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
>
> On Mon, May 6, 2019 at 5:24 PM Eugeniu Rosca wrote:
> > On Mon, May 06, 2019 at 03:47:05PM +0200, Simon Horman wrote:
> > > On Sat, May 04, 2019 at 02:42:53AM +0200, Eugeniu Rosca wrote:
> > > > Starting with v4.15-rc2 commit ad67b74d2469d9 ("printk: hash addresses
> > > > printed with %p"), enabling debug prints in sh-sci.c would generate
> > > > output like below confusing the users who try to sneak into the
> > > > internals of the driver:
> > > >
> > > > sh-sci e6e88000.serial: sci_request_dma: TX: got channel
> > > > (ptrval)
> > > > sh-sci e6e88000.serial: sci_request_dma: mapped 4096@(ptrval)
> > > > to 0x0006798bf000
> > > > sh-sci e6e88000.serial: sci_request_dma: RX: got channel
> > > > (ptrval)
> > > > sh-sci e6e88000.serial: sci_dma_tx_work_fn: (ptrval): 0...2,
> > > > cookie 2
> > > >
> > > > There are two possible fixes for that:
> > > > - get rid of '%p' prints if they don't reveal any useful information
> > > > - s/%p/%px/, since it is unlikely we have any concerns leaking the
> > > >pointer values when running a debug/non-production kernel
> > >
> > > I am concerned that this may expose information in circumstances
> > > where it is undesirable. Is it generally accepted practice to
> > > use %px in conjunction with dev_dbg() ?
> > >
> > > ...
> >
> > Below commits performed a similar s/%p/%px/ update in debug context:
> >
> > Authors (CC-ed) Commit Subject
> >
> > Christophe Leroy b18f0ae92b0a1d ("powerpc/prom: fix early DEBUG messages")
> > Helge Deller 3847dab7742186 ("parisc: Add alternative coding
> > infrastructure")
> > Michael Neuling 51c3c62b58b357 ("powerpc: Avoid code patching freed init
> > sections")
> > Kuninori Morimoto dabdbe3ae0cb9a ("ASoC: rsnd: don't use %p for dev_dbg()")
> > Philip Yang fa7e65147e5dca ("drm/amdkfd: use %px to print user space
> > address instead of %p")
> > Matthew Wilcox68c1f08203f2b0 ("lib/list_debug.c: print unmangled
> > addresses")
> > Borislav Petkov 0e6c16c652cada ("x86/alternative: Print unadorned
> > pointers")
> > Darrick J. Wong c96900435fa9fd ("xfs: use %px for data pointers when
> > debugging")
> > Helge Deller 04903c06b4854d ("parisc: Show unhashed HPA of Dino chip")
> >
> > To quote Matthew, with respect to any debug prints:
> > If an attacker can force this message to be printed, we've already lost.
>
> I think the issue with using %px in debug code is that a distro may enable
> CONFIG_DYNAMIC_DEBUG (it is enabled in several defconfigs), after which
> an attacker just has to convince/trick the system into enabling debug for that
> particular driver.
How about going the route of commit c96900435fa9fd ("xfs: use %px for
data pointers when debugging"), i.e. s/%p/"PTR_FMT"/ like below (this
would enable the expected debug output only on manually defining DEBUG
in the *.c file, while still keeping the output hashed on
DYNAMIC_DEBUG=y if DEBUG is undefined).
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 3cd139752d3f..69cd87c5ef0c 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -56,6 +56,12 @@
#include
#endif
+#ifdef DEBUG
+#define PTR_FMT "%px"
+#else
+#define PTR_FMT "%p"
+#endif
+
#include "serial_mctrl_gpio.h"
#include "sh-sci.h"
@@ -1434,7 +1440,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
goto switch_to_pio;
}
- dev_dbg(port->dev, "%s: %p: %d...%d, cookie %d\n",
+ dev_dbg(port->dev, "%s: "PTR_FMT": %d...%d, cookie %d\n",
__func__, xmit->buf, xmit->tail, xmit->head, s->cookie_tx);
dma_async_issue_pending(chan);
>
> > In any case, I won't be affected much if the change is not accepted,
> > since it doesn't resolve any major issue on my end. Thanks!
>
> OK.
>
> 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 1/6] serial: sh-sci: Reveal ptrval in dev_dbg
On Mon, May 06, 2019 at 03:47:05PM +0200, Simon Horman wrote:
> On Sat, May 04, 2019 at 02:42:53AM +0200, Eugeniu Rosca wrote:
> > Starting with v4.15-rc2 commit ad67b74d2469d9 ("printk: hash addresses
> > printed with %p"), enabling debug prints in sh-sci.c would generate
> > output like below confusing the users who try to sneak into the
> > internals of the driver:
> >
> > sh-sci e6e88000.serial: sci_request_dma: TX: got channel (ptrval)
> > sh-sci e6e88000.serial: sci_request_dma: mapped 4096@(ptrval) to
> > 0x0006798bf000
> > sh-sci e6e88000.serial: sci_request_dma: RX: got channel (ptrval)
> > sh-sci e6e88000.serial: sci_dma_tx_work_fn: (ptrval): 0...2, cookie
> > 2
> >
> > There are two possible fixes for that:
> > - get rid of '%p' prints if they don't reveal any useful information
> > - s/%p/%px/, since it is unlikely we have any concerns leaking the
> >pointer values when running a debug/non-production kernel
>
> I am concerned that this may expose information in circumstances
> where it is undesirable. Is it generally accepted practice to
> use %px in conjunction with dev_dbg() ?
>
> ...
Below commits performed a similar s/%p/%px/ update in debug context:
Authors (CC-ed) Commit Subject
Christophe Leroy b18f0ae92b0a1d ("powerpc/prom: fix early DEBUG messages")
Helge Deller 3847dab7742186 ("parisc: Add alternative coding
infrastructure")
Michael Neuling 51c3c62b58b357 ("powerpc: Avoid code patching freed init
sections")
Kuninori Morimoto dabdbe3ae0cb9a ("ASoC: rsnd: don't use %p for dev_dbg()")
Philip Yang fa7e65147e5dca ("drm/amdkfd: use %px to print user space
address instead of %p")
Matthew Wilcox68c1f08203f2b0 ("lib/list_debug.c: print unmangled addresses")
Borislav Petkov 0e6c16c652cada ("x86/alternative: Print unadorned pointers")
Darrick J. Wong c96900435fa9fd ("xfs: use %px for data pointers when
debugging")
Helge Deller 04903c06b4854d ("parisc: Show unhashed HPA of Dino chip")
To quote Matthew, with respect to any debug prints:
If an attacker can force this message to be printed, we've already lost.
In any case, I won't be affected much if the change is not accepted,
since it doesn't resolve any major issue on my end. Thanks!
--
Best Regards,
Eugeniu.
[PATCH 6/6] Revert "arm64: dts: renesas: r8a77995: add DMA for SCIF2"
This reverts commit af2ea3df851ffa68ad07ff59d4dabadbf33b45ef.
Both empirically and based on the existing documentation, it looks like
SCIF2 does not support DMA. Let's disable it. Full reasoning is given in
commit ("Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"").
Fixes: af2ea3df851ffa ("arm64: dts: renesas: r8a77995: add DMA for SCIF2")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77995.dtsi | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
index 5bf3af246e14..bd3ac5d00b2e 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
@@ -740,9 +740,6 @@
<&cpg CPG_CORE R8A77995_CLK_S3D1C>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
- dmas = <&dmac1 0x13>, <&dmac1 0x12>,
- <&dmac2 0x13>, <&dmac2 0x12>;
- dma-names = "tx", "rx", "tx", "rx";
power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
resets = <&cpg 310>;
status = "disabled";
--
2.21.0
[PATCH 1/6] serial: sh-sci: Reveal ptrval in dev_dbg
Starting with v4.15-rc2 commit ad67b74d2469d9 ("printk: hash addresses
printed with %p"), enabling debug prints in sh-sci.c would generate
output like below confusing the users who try to sneak into the
internals of the driver:
sh-sci e6e88000.serial: sci_request_dma: TX: got channel (ptrval)
sh-sci e6e88000.serial: sci_request_dma: mapped 4096@(ptrval) to
0x0006798bf000
sh-sci e6e88000.serial: sci_request_dma: RX: got channel (ptrval)
sh-sci e6e88000.serial: sci_dma_tx_work_fn: (ptrval): 0...2, cookie 2
There are two possible fixes for that:
- get rid of '%p' prints if they don't reveal any useful information
- s/%p/%px/, since it is unlikely we have any concerns leaking the
pointer values when running a debug/non-production kernel
Go second route to preserve original debug output and minimize the diff.
Fixes: ad67b74d2469d9 ("printk: hash addresses printed with %p")
Signed-off-by: Eugeniu Rosca
---
drivers/tty/serial/sh-sci.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 3cd139752d3f..82660f8e6d86 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1434,7 +1434,7 @@ static void sci_dma_tx_work_fn(struct work_struct *work)
goto switch_to_pio;
}
- dev_dbg(port->dev, "%s: %p: %d...%d, cookie %d\n",
+ dev_dbg(port->dev, "%s: %px: %d...%d, cookie %d\n",
__func__, xmit->buf, xmit->tail, xmit->head, s->cookie_tx);
dma_async_issue_pending(chan);
@@ -1570,7 +1570,7 @@ static void sci_request_dma(struct uart_port *port)
return;
chan = sci_request_dma_chan(port, DMA_MEM_TO_DEV);
- dev_dbg(port->dev, "%s: TX: got channel %p\n", __func__, chan);
+ dev_dbg(port->dev, "%s: TX: got channel %px\n", __func__, chan);
if (chan) {
/* UART circular tx buffer is an aligned page. */
s->tx_dma_addr = dma_map_single(chan->device->dev,
@@ -1581,7 +1581,7 @@ static void sci_request_dma(struct uart_port *port)
dev_warn(port->dev, "Failed mapping Tx DMA
descriptor\n");
dma_release_channel(chan);
} else {
- dev_dbg(port->dev, "%s: mapped %lu@%p to %pad\n",
+ dev_dbg(port->dev, "%s: mapped %lu@%px to %pad\n",
__func__, UART_XMIT_SIZE,
port->state->xmit.buf, &s->tx_dma_addr);
@@ -1591,7 +1591,7 @@ static void sci_request_dma(struct uart_port *port)
}
chan = sci_request_dma_chan(port, DMA_DEV_TO_MEM);
- dev_dbg(port->dev, "%s: RX: got channel %p\n", __func__, chan);
+ dev_dbg(port->dev, "%s: RX: got channel %px\n", __func__, chan);
if (chan) {
unsigned int i;
dma_addr_t dma;
--
2.21.0
[PATCH 5/6] Revert "arm64: dts: renesas: r8a77990: Enable DMA for SCIF2"
This reverts commit a99de47921565c233092b0d3c4652b3a10e715ec.
Both empirically and based on the existing documentation, it looks like
SCIF2 does not support DMA. Let's disable it. Full reasoning is given in
commit ("Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"").
Fixes: a99de47921565c ("arm64: dts: renesas: r8a77990: Enable DMA for SCIF2")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77990.dtsi | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
index d2ad665fe2d9..4bf32bfaafc0 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
@@ -1018,9 +1018,7 @@
<&cpg CPG_CORE R8A77990_CLK_S3D1C>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
- dmas = <&dmac1 0x13>, <&dmac1 0x12>,
- <&dmac2 0x13>, <&dmac2 0x12>;
- dma-names = "tx", "rx", "tx", "rx";
+
power-domains = <&sysc R8A77990_PD_ALWAYS_ON>;
resets = <&cpg 310>;
status = "disabled";
--
2.21.0
[PATCH 2/6] Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"
This reverts commit 97f26702bc95b5c3a72671d5c6675e4d6ee0a2f4.
Here is the story behind this revert.
Mainline commit [0] landed in the stable tree as commit [1], from where
it reached us in the form of regular stable update. After that, Michael
started to report occasional (30-50%) freezes of serial console on
booting M3-ES1.1-Salvator-XS. Same happened on M3-ES1.1-Salvator-X.
Every time the issue occurs, the serial console outputs below [2]
before becoming totally unresponsive and printing nothing else:
rcar-dmac e730.dma-controller: Channel Address Error
Git bisecting shows that the problem is contributed by commits [0-1].
While we can't be 100% certain (since we don't have the SCIF design docs
revealing its internal implementation detail) we think there is plenty
of evidence to assume that DMA is not supported on SCIF2, hence should
stay disabled on this specific channel:
- Excerpt from Chapter 17. Direct Memory Access Controller for System
(SYS-DMAC) of R19UH0105EJ0150 Rev.1.50:
-8<-
[H3, H3-N, M3-W, V3M, V3H, D3, M3-N, E3]
The following modules can issue on-chip peripheral module requests.
[..] HSCIF0/1/2/3/4, [..] SCIF0/1/3/4/5,
-8<-
- Excerpt from RENESAS_RCH3M3M3NE3_SCIF_UME_v2.00.pdf (Yocto v3.15.0):
-8<-
DMA Transfer:
- Support: SCIF0, SCIF1, SCIF3, SCIF4, SCIF5
- Not support: SCIF2
-8<-
- Disabled SCIF2 DMA in official Renesas v4.9/v4.14 kernels, e.g. see:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=e79c418fda8c
Based on the issues generated by [0-1] (reproduced on H3, M3 and M3N)
and the doc statements presented above, we think it makes sense to
disable DMA on SCIF2 for most/all R-Car3 SoCs.
[0] v5.0-rc6 commit 97f26702bc95b5 ("arm64: dts: renesas: r8a7796: Enable DMA
for SCIF2")
[1] v4.14.106 commit 703db5d1b1759f ("arm64: dts: renesas: r8a7796: Enable DMA
for SCIF2")
[2] scif (DEBUG) and rcar-dmac logs:
https://gist.github.com/erosca/132cce76a619724a9e4fa61d1db88c66
Fixes: 97f26702bc95b5 ("arm64: dts: renesas: r8a7796: Enable DMA for SCIF2")
Reported-by: Michael Rodin
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index cdf784899cf8..23de63f3d6c3 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -1262,9 +1262,6 @@
<&cpg CPG_CORE R8A7796_CLK_S3D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
- dmas = <&dmac1 0x13>, <&dmac1 0x12>,
- <&dmac2 0x13>, <&dmac2 0x12>;
- dma-names = "tx", "rx", "tx", "rx";
power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
resets = <&cpg 310>;
status = "disabled";
--
2.21.0
[PATCH 0/6] Zap SCIF2 DMA configuration in R-Car Gen3 DTS
This series is triggered by a regression on M3 targets caused by
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=703db5d1b175
("arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"), when applied
on top of rcar-3.9.x Renesas official kernel.
This collection of patches attempts to consistently propagate the fix
across the existing R-Car3 DTS. Full story is placed into
commit ("Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"").
While debugging drivers/tty/serial/sh-sci.c, a minor update avoiding
__ptrval__ in dev_dbg() is included here as well.
Tested using v5.1-rc7-131-gea9866793d1e on:
- H3-ES2.0-ULCB
- M3N-ES1.1-ULCB
- M3-ES1.1-Salvator-XS
Eugeniu Rosca (6):
serial: sh-sci: Reveal ptrval in dev_dbg
Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"
arm64: dts: renesas: r8a7795: zap dma configuration in scif2
Revert "arm64: dts: renesas: r8a77965: Enable DMA for SCIF2"
Revert "arm64: dts: renesas: r8a77990: Enable DMA for SCIF2"
Revert "arm64: dts: renesas: r8a77995: add DMA for SCIF2"
arch/arm64/boot/dts/renesas/r8a7795.dtsi | 3 ---
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 3 ---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 3 ---
arch/arm64/boot/dts/renesas/r8a77990.dtsi | 4 +---
arch/arm64/boot/dts/renesas/r8a77995.dtsi | 3 ---
drivers/tty/serial/sh-sci.c | 8
6 files changed, 5 insertions(+), 19 deletions(-)
--
2.21.0
[PATCH 3/6] arm64: dts: renesas: r8a7795: zap dma configuration in scif2
Both empirically and based on the existing documentation, it looks like
SCIF2 does not support DMA. Let's disable it. Full reasoning is given in
commit ("Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"").
Fixes: eb21089c32054e ("arm64: dts: renesas: r8a7795: Add missing SYS-DMAC2
dmas")
Fixes: 49af46b4095672 ("arm64: renesas: r8a7795: Add all SCIF nodes")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a7795.dtsi | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index abeac3059383..7704bd46afdf 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1323,9 +1323,6 @@
<&cpg CPG_CORE R8A7795_CLK_S3D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
- dmas = <&dmac1 0x13>, <&dmac1 0x12>,
- <&dmac2 0x13>, <&dmac2 0x12>;
- dma-names = "tx", "rx", "tx", "rx";
power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
resets = <&cpg 310>;
status = "disabled";
--
2.21.0
[PATCH 4/6] Revert "arm64: dts: renesas: r8a77965: Enable DMA for SCIF2"
This reverts commit 05c8478abd485507c25aa565afab604af8d8fe46.
Both empirically and based on the existing documentation, it looks like
SCIF2 does not support DMA. Let's disable it. Full reasoning is given in
commit ("Revert "arm64: dts: renesas: r8a7796: Enable DMA for SCIF2"").
Fixes: 05c8478abd4855 ("arm64: dts: renesas: r8a77965: Enable DMA for SCIF2")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 9763d108e183..979f14d1fcc4 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1068,9 +1068,6 @@
<&cpg CPG_CORE R8A77965_CLK_S3D1>,
<&scif_clk>;
clock-names = "fck", "brg_int", "scif_clk";
- dmas = <&dmac1 0x13>, <&dmac1 0x12>,
- <&dmac2 0x13>, <&dmac2 0x12>;
- dma-names = "tx", "rx", "tx", "rx";
power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
resets = <&cpg 310>;
status = "disabled";
--
2.21.0
Re: [PATCH] arm64: dts: ulcb-kf: Add support for TI WL1837
Hi Simon, On Mon, Apr 29, 2019 at 10:17:48AM +0200, Simon Horman wrote: > Hi again, > > I have been able to solicit a limited private review of this patch and > have gone ahead and applied it for inclusion in v5.2. Thank you. In case of any concerns/reports/fixes applicable to this patch, we would appreciate if you CC ADIT people. We will then reply with best possible latency, since we are interested in having a working WiFi/BT on ULCB-KF. -- Best Regards, Eugeniu.
Re: [PATCH] arm64: dts: ulcb-kf: Add support for TI WL1837
Hi Simon,
Do we have any chance getting this upstream? If so, would you kindly
list the acceptance criteria we have to conform to?
Many thanks.
On Thu, Apr 11, 2019 at 02:41:03PM +0200, Spyridon Papageorgiou wrote:
> This patch adds description of TI WL1837 and links interfaces
> to communicate with the IC, namely the SDIO interface to WLAN.
>
> Signed-off-by: Spyridon Papageorgiou
> ---
> arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 49
>
> 1 file changed, 49 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> index 7a09576..27851a7 100644
> --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> @@ -38,6 +38,18 @@
> regulator-min-microvolt = <500>;
> regulator-max-microvolt = <500>;
> };
> +
> + wlan_en: regulator-wlan_en {
> + compatible = "regulator-fixed";
> + regulator-name = "wlan-en-regulator";
> +
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> + startup-delay-us = <7>;
> + enable-active-high;
> + };
> };
>
> &can0 {
> @@ -88,6 +100,13 @@
> line-name = "Audio_Out_OFF";
> };
>
> + sd-wifi-mux {
> + gpio-hog;
> + gpios = <5 GPIO_ACTIVE_HIGH>;
> + output-low; /* Connect WL1837 */
> + line-name = "SD WiFi mux";
> + };
> +
> hub_pwen {
> gpio-hog;
> gpios = <6 GPIO_ACTIVE_HIGH>;
> @@ -254,6 +273,12 @@
> function = "scif1";
> };
>
> + sdhi3_pins: sdhi3 {
> + groups = "sdhi3_data4", "sdhi3_ctrl";
> + function = "sdhi3";
> + power-source = <3300>;
> + };
> +
> usb0_pins: usb0 {
> groups = "usb0";
> function = "usb0";
> @@ -273,6 +298,30 @@
> status = "okay";
> };
>
> +&sdhi3 {
> + pinctrl-0 = <&sdhi3_pins>;
> + pinctrl-names = "default";
> +
> + vmmc-supply = <&wlan_en>;
> + vqmmc-supply = <&wlan_en>;
> + bus-width = <4>;
> + no-1-8-v;
> + non-removable;
> + cap-power-off-card;
> + keep-power-in-suspend;
> + max-frequency = <2600>;
> + status = "okay";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + wlcore: wlcore@2 {
> + compatible = "ti,wl1837";
> + reg = <2>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> + };
> +};
> +
> &usb2_phy0 {
> pinctrl-0 = <&usb0_pins>;
> pinctrl-names = "default";
> --
> 2.7.4
>
--
Best regards,
Eugeniu.
Re: [PATCH v4 12/12] staging: most: Documentation: update driver documentation
Hi Chris, We really appreciate your feedback. Thanks for that. For us going the vanilla way roughly means backporting to v4.14: - v5.1-rc5 MOST state (easy/straightforward) - MOST patches from either https://github.com/CogentEmbedded/meta-rcar/ or https://github.com/microchip-ais/meta-medialb-rcar - A number of patches from mld-1.8.0 >From the above list, the last step looks a bit more painful, but we will try to deal with that. If there are any mainline-relevant fixes developed on the way, we hope to find some time to submit them to you and get feedback. Best regards, Eugeniu. On Wed, Apr 17, 2019 at 01:45:41PM +, christian.gr...@microchip.com wrote: > On Mo, 2019-04-15 at 23:21 +0200, Eugeniu Rosca wrote: > > External E-Mail > > > > > > Hello Christian, hello Andrey, > > cc: Vladimir Barinov > > cc: linux-renesas-soc > > > > Our team currently has the requirement of adding MOST support to > > the v4.14-based R-Car3 kernel [1], matching the level of features > > and interfaces of mld-1.8.0 [2] release. > > > > Based on a quick check [3], the mld-1.8.0 release contains 221 non- > > merge > > non-mainline commits. It seems like at least some (most?) of them > > have > > been reworked during upstreaming and are now part of vanilla, thanks > > to > > your efforts. > > > > Since you've actively participated in pushing the out-of-tree drivers > > to mainline, could you please share your gut feeling whether the > > current mainline state of the MOST subsystem matches the mld-1.8.0 > > release in terms of features and interfaces? > > > > No, it doesn't. The version upstream does not have all the bells > and whistles the mld-1.8.0 version has, yet. Focus of the latest > mainline commits was on the driver model and the switch to configfs. > > > At first glance, such mld-1.8.0 functionalities like "flexible PCM > > rate support" [4], as well as a number of dim2 sysfs entries [5-8] > > appear to be missing in v5.1-rc5. Could you please feedback if these > > have been deliberately dropped or didn't make their way for another > > reason? What would be your recommendation between going the mld-1.8.0 > > and going the v5.1-rc5 way for MOST? > > > > The functionalities you're referring to have _not_ intentionally been > dropped. They will find their way into mainline. If there are urgent > feature requests, we could prioritize them. > > Bottom line is, the upstream version is the one you should be using, > as it is going to replace the mld-1.x branch. This is exactly what we > will soon be doing for AGL by the way. > > thanks, > Chris
Re: [PATCH v4 12/12] staging: most: Documentation: update driver documentation
Hello Christian, hello Andrey,
cc: Vladimir Barinov
cc: linux-renesas-soc
Our team currently has the requirement of adding MOST support to
the v4.14-based R-Car3 kernel [1], matching the level of features
and interfaces of mld-1.8.0 [2] release.
Based on a quick check [3], the mld-1.8.0 release contains 221 non-merge
non-mainline commits. It seems like at least some (most?) of them have
been reworked during upstreaming and are now part of vanilla, thanks to
your efforts.
Since you've actively participated in pushing the out-of-tree drivers
to mainline, could you please share your gut feeling whether the
current mainline state of the MOST subsystem matches the mld-1.8.0
release in terms of features and interfaces?
At first glance, such mld-1.8.0 functionalities like "flexible PCM
rate support" [4], as well as a number of dim2 sysfs entries [5-8]
appear to be missing in v5.1-rc5. Could you please feedback if these
have been deliberately dropped or didn't make their way for another
reason? What would be your recommendation between going the mld-1.8.0
and going the v5.1-rc5 way for MOST?
Best regards,
Eugeniu.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/
[2] https://github.com/microchip-ais/linux/releases/tag/mld-1.8.0
[3] git rev-list --no-merges --count \
$(git merge-base linux/master mld-1.8.0)..mld-1.8.0
221
[4] https://github.com/microchip-ais/linux/commit/8821bad23ff2
("staging: most: aim-sound: add flexible PCM rate support")
[5] https://github.com/microchip-ais/linux/commit/5030f77717a1d7
("staging: most: dim2: add sysfs property dci/node_position")
[6] https://github.com/microchip-ais/linux/commit/3a840d7b504552
("staging: most: dim2: add sysfs property dci/node_address")
[7] https://github.com/microchip-ais/linux/commit/0494ccb4bc5f39
("staging: most: dim2: add sysfs property dci/ni_state")
[8] https://github.com/microchip-ais/linux/commit/3198a5c5f0663
("staging: most: dim2: add sysfs property dci/mep_eui48")
Re: [PATCH v2] serial: sh-sci: Support for HSCIF RX sampling point adjustment
Dear Renesas Open Source team, We've recently switched from rcar-3.7.x to rcar-3.9.x [1] kernel and the latter contains this patch [2] by virtue of rcar-3.9.0 commit [3], which mirrors v4.18-rc1 commit [4] in mainline. JFYI, quite far away in the delivery chain, we've received below report: > With this patch [2-4] there are reports about broken data > communication with 115200 baud with SXM module. Reverting > this patch results in successful communication, again. While this scarce information barely helps anybody, I thought that sharing it with you might be beneficial in case you collect several reports linked to this specific commit in future, meaning it potentially adds a regression. Also, if you are aware of any userland changes that might be required/assumed by this patch or in case you have any alternative ideas how to avoid reverting this patch, your feedback would be very appreciated. [1] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/tag/?h=rcar-3.9.3 [2] https://patchwork.kernel.org/patch/10322809/ [3] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?id=e395fc813539 [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=63ba1e00f178 Best regards, Eugeniu.
Re: [PATCH 1/2] i2c: rcar: refactor private flags
Hi Wolfram,
On Fri, Oct 05, 2018 at 06:27:28PM +0200, Wolfram Sang wrote:
>
> > May I ask how exactly you spotted the "shift-31-problem" in
> > drivers/i2c/busses/i2c-rcar.c:
> > - visual code review?
> > - static analysis, special compiler flags?
>
> This one. I run a set of static code analyziers when applying patches.
> One of them is 'cppcheck' which reported it.
Indeed, cppcheck reports w/o this patch:
[drivers/i2c/busses/i2c-rcar.c:972]: (error) Shifting signed 32-bit value by 31
bits is undefined behaviour
[drivers/i2c/busses/i2c-rcar.c:1008]: (error) Shifting signed 32-bit value by
31 bits is undefined behaviour
>
> > According to feedback from GCC community [2], with 'gcc -std=gnu89',
> > shifting into (not past) the sign bit is "defined behavior" which is why
> > UBSAN doesn't report this as an issue in Linux kernel. That makes me
>
> I see. I guess it can be argued. Yet, BIT() solves other issues as well
> ('1' vs '1u'), so this was probably a reasonable move nonetheless, plus
> we are super-super-sure about the shifting now.
>
I agree. There is no doubt that avoiding/fixing shifting into the sign
bit makes the code more portable and will lessen the pain when
switching Kbuild to C99/C11 (if ever needed). I still have open
questions, but since they go beyond i2c framework and beyond kernel
itself (as said, they originate from porting UBSan to U-Boot), I will
discuss them elsewhere.
Thanks again for the reply.
Best regards,
Eugeniu.
Re: [PATCH 1/2] i2c: rcar: refactor private flags
Hi Wolfram,
> On August 8, 2018 at 9:59 AM Wolfram Sang
> wrote:
>
> Use BIT macro to avoid shift-31-problem,
May I ask how exactly you spotted the "shift-31-problem" in
drivers/i2c/busses/i2c-rcar.c:
- visual code review?
- static analysis, special compiler flags?
- run-time testing with UBSAN=y?
The background of my question is seeing a lot of UBSAN errors caused
by (1 << 31) in U-Boot with UBSAN=y [1]. Based on my experiments, the
Linux UBSAN simply doesn't complain about (1 << 31). I traced the
difference to be caused by the -std=gnu{89,11} value passed to gcc,
which varies between U-Boot and Linux.
According to feedback from GCC community [2], with 'gcc -std=gnu89',
shifting into (not past) the sign bit is "defined behavior" which is why
UBSAN doesn't report this as an issue in Linux kernel. That makes me
curious about the background of this patch, since it might shed some
light onto how to further handle the (1 << 31) UBSAN warnings in
U-Boot.
Thank you very much for your feedback.
[1] https://patchwork.ozlabs.org/cover/962307/
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87392
Best regards,
Eugeniu.
Re: [PATCH v3 net-next] dt-bindings: can: rcar_can: document r8a77965 support
Hi Marc, On Mon, Aug 27, 2018 at 12:08:48PM +0200, Marc Kleine-Budde wrote: > On 08/20/2018 04:49 PM, Eugeniu Rosca wrote: > > From: Eugeniu Rosca > > > > Document the support for rcar_can on R8A77965 SoC devices. > > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > > "assigned-clock-rates" properties (thanks, Sergei). > > > > Signed-off-by: Eugeniu Rosca > > Reviewed-by: Simon Horman > > Reviewed-by: Kieran Bingham > > Added to linux-can. I can't find the patch in below repositories: - https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git - https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git Could you please let me know what "linux-can" means? > > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions| Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917- | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | Thanks, Eugeniu.
Re: [PATCH v2 6/7] arm64: dts: renesas: r8a77965: Add HSCIF0 device node
Hi Simon,
On Wed, Aug 22, 2018 at 01:12:47PM +0200, Simon Horman wrote:
> On Sun, Aug 12, 2018 at 03:31:48PM +0200, Eugeniu Rosca wrote:
> > Fix below DTC error:
> > Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:36.1-8 Label or path hscif0
> > not found
> >
> > The DTC error occurs *after* the addition of r8a77965-m3nulcb-kf.dts.
> > Fix it beforehand.
> >
> > Inspired from v4.12-rc1 commits:
> > - commit 68cd16107260 ("arm64: dts: r8a7796 dtsi: Add all HSCIF nodes")
> > - commit 6d50bb893504 ("arm64: dts: r8a7796: Enable HSCIF DMA")
> > - commit bec0948e810f ("arm64: dts: r8a7796: Add reset control properties")
> >
> > Signed-off-by: Eugeniu Rosca
> > ---
> > Changes in v2:
> > - Slightly improved the commit description.
> > - Note that this commit could be replaced by
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git/commit/?h=b8e3c8e17611
>
> That commit has been accepted for upstream so it should be used.
Thanks for the explanation. No further questions/concerns.
Best regards,
Eugeniu.
Re: [PATCH v2 3/7] pinctrl: sh-pfc: r8a77965: Add HSCIF0 pins, groups, and functions
Hi Geert,
On Mon, Aug 27, 2018 at 05:07:39PM +0200, Geert Uytterhoeven wrote:
> Hi Eugeniu,
>
> On Sun, Aug 12, 2018 at 3:33 PM Eugeniu Rosca wrote:
> > According to R-Car Gen3 HW manual Rev.1.00 Apr 2018, M3-N SoC implements
> > five (0..4) HSCIF channels, similar to H3, M3-W and E3.
> >
> > The story behind this patch is tackling below dmesg warnings, which pop
> > up when booting M3NULCB Kingfisher board:
> >
> > $ dmesg | grep sh-pfc
> > sh-pfc e606.pin-controller: r8a77965_pfc support registered
> > sh-pfc e606.pin-controller: function 'hscif0' not supported
> > sh-pfc e606.pin-controller: invalid function hscif0 in map table
> > sh-pfc e606.pin-controller: function 'hscif0' not supported
> > sh-pfc e606.pin-controller: invalid function hscif0 in map table
> >
> > To fix them, extract the HSCIF0 part from below v4.15-rc1 commits:
> > - commit 7a362e3488cb ("pinctrl: sh-pfc: r8a7795: Add HSCIF pins, groups,
> > and functions")
> > - commit 0e4e4999aac1 ("pinctrl: sh-pfc: r8a7796: Add HSCIF pins, groups,
> > and functions")
> >
> > Note that `checkpatch --strict` throws several "CHECK: Please use a
> > blank line after function/struct/union/enum declarations", which are
> > ignored for the sake of staying in sync with the aforementioned commits.
> >
> > Signed-off-by: Eugeniu Rosca
>
> Thanks for your patch, but please see commit 7628fa811b8af571 ("pinctrl:
> sh-pfc: r8a77965: Add HSCIF pins, groups, and functions") in v4.19-rc1.
Thanks for pointing out. Please, feel free to drop the patch. Its
purpose was to build a self-contained (no new compiler/dmesg
warnings/errors) patch series assuming v4.18.
> 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 5/7] arm64: dts: renesas: r8a77965: Add CAN{0,1} placeholder nodes
Hi Simon, hi Geert,
On Mon, Aug 27, 2018 at 02:44:47PM +0200, Simon Horman wrote:
> On Thu, Aug 23, 2018 at 10:52:09AM +0200, Geert Uytterhoeven wrote:
> > On Fri, Aug 17, 2018 at 3:53 PM Kieran Bingham
> > wrote:
> > > On 12/08/18 14:31, Eugeniu Rosca wrote:
> > > > According to R-Car Gen3 HW manual rev1.00, R-Car M3-N has two CAN
> > > > interfaces, similar to H3, M3-W and other SoCs from the same family.
> > > >
> > > > Add CAN placeholder nodes to avoid below DTC errors:
> > > > 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
> > > >
> > > > These errors occur *after* the addition of r8a77965-m3nulcb-kf.dts.
> > > > Fix them beforehand.
> > > >
> > > > 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
> > >
> > > Reviewed-by: Kieran Bingham
> > >
> > >
> > > > ---
> > > > Changes in v2:
> > > > - [Kieran Bingham] Improved commit description:
> > > >- Referenced the newer HW manual rev1.00 instead of rev0.55E.
> > > >- Kept the "true story" behind the patch. Just made it more clear.
> > > > - [Geert Uytterhoeven] Replaced CAN0 and CAN1 nodes with placeholders
> > > >(no CAN testing was done to validate the DTS configuration).
> > > > ---
> > > > arch/arm64/boot/dts/renesas/r8a77965.dtsi | 16
> > > > 1 file changed, 16 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > index 486aecacb22a..4da479d3c226 100644
> > > > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > > @@ -656,6 +656,22 @@
> > > > status = "disabled";
> > > > };
> > > >
> > > > + can0: can@e6c3 {
> > > > + compatible = "renesas,can-r8a77965",
> > > > + "renesas,rcar-gen3-can";
> > > > + reg = <0 0xe6c3 0 0x1000>;
> > > > + /* placeholder */
> > > > + status = "disabled";
> > > > + };
> > >
> > > This is probably more detail than is needed for a placeholder, but it
> > > looks correct so I think this is fine.
> >
> > Indeed. Adding the "compatible" properties means they're no longer
> > placeholders, and will be probed by the driver, possibly leading to
> > undefined behavior.
> >
> > Hence please limit the placeholders to the absolute required minimum,
> > and thus drop the "compatible" and "status" properties.
>
> Agreed, I will update the patch accordingly.
My understanding is that Simon will drop the compatible strings and no
action is needed from my end. Let me know otherwise.
Thanks,
Eugeniu.
Re: [PATCH v2 5/7] arm64: dts: renesas: r8a77965: Add CAN{0,1} placeholder nodes
Hi Kieran, I appreciate the detailed reply zooming in into many aspects (both technical and related to process/workflow) of contributing/reviewing patches. I take it as is. I could elaborate on specific parts of it, like applying the "undefined behavior" term (which comes from the world of compilers) to the software we write. This doesn't sound right to me, since our software is guided by our own requirements and one of this requirements is (should be) predictable and safe behavior given some junky/incomplete DTS configuration. If any bugs exist dealing with nonsense configuration, IMHO they should be fixed in the driver. Since my purpose is simply seeing the patches in upstream the way maintainers like them best, I will put little to no time discussing this and will mainly focus on submitting any changes requested by the reviewers. Thanks, Eugeniu.
Re: [PATCH v2 5/7] arm64: dts: renesas: r8a77965: Add CAN{0,1} placeholder nodes
Dear reviewers,
On Thu, Aug 23, 2018 at 11:01:46AM +0200, Geert Uytterhoeven wrote:
> Hi Sergei,
>
> On Thu, Aug 23, 2018 at 10:56 AM Sergei Shtylyov
> wrote:
> > On 8/23/2018 11:52 AM, Geert Uytterhoeven wrote:
> > >>> According to R-Car Gen3 HW manual rev1.00, R-Car M3-N has two CAN
> > >>> interfaces, similar to H3, M3-W and other SoCs from the same family.
> > >>>
> > >>> Add CAN placeholder nodes to avoid below DTC errors:
> > >>> 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
> > >>>
> > >>> These errors occur *after* the addition of r8a77965-m3nulcb-kf.dts.
> > >>> Fix them beforehand.
> > >>>
> > >>> 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
>
> > >>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > >>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > >>> @@ -656,6 +656,22 @@
> > >>>status = "disabled";
> > >>>};
> > >>>
> > >>> + can0: can@e6c3 {
> > >>> + compatible = "renesas,can-r8a77965",
> > >>> + "renesas,rcar-gen3-can";
> > >>> + reg = <0 0xe6c3 0 0x1000>;
> > >>> + /* placeholder */
> > >>> + status = "disabled";
> > >>> + };
> > >>
> > >> This is probably more detail than is needed for a placeholder, but it
> > >> looks correct so I think this is fine.
> > >
> > > Indeed. Adding the "compatible" properties means they're no longer
> > > placeholders, and will be probed by the driver, possibly leading to
> > > undefined behavior.
> >
> > I don't think the disabled device nodes are actually probed.
>
> They will be by ulcb-kf.dtsi, after the addition of
> r8a77965-m3nulcb-kf.dts, cfr.
> the errors and rationale documented in the commit message.
I took some time to examine the "52. Controller Area Network Interface
(CAN interface)" chapter of HW SoC manual rev1.00 in detail and there is
no difference mentioned between the SoCs (H3, M3-W, M3-N, D3, E3) which
implement the two CAN (non-FD) interfaces. This is confirmed by the
perfectly symmetrical can{0,1} configuration present in the H3,
M3-W and D3 device tree sources:
$ git grep -l can0 -- arch/arm64/boot/dts/renesas/r8*dtsi
arch/arm64/boot/dts/renesas/r8a7795.dtsi
arch/arm64/boot/dts/renesas/r8a7796.dtsi
arch/arm64/boot/dts/renesas/r8a77995.dtsi
So, to be honest, in my opinion, besides consuming time arguing about
what a placeholder DTS node is (btw, commits [1] and [2] do include a
compatible string while adding a "placeholder" node), we also force
users to 'git blame' multiple times to reconstruct the history of CAN
controller nodes on M3-N, while for H3, M3-W and D3 a single commit was
enough to add the functionality.
That said, I don't see any dmesg differences on M3NULCB between having
and not having the compatible string in the can{0,1} nodes.
> Hence please limit the placeholders to the absolute required minimum,
> and thus drop the "compatible" and "status" properties.
My understanding is that the lack of status is equivalent with
'status = "okay"' (i.e. enable the node), so I don't really see why
'status = "disabled"' should hurt for a placeholder, especially seeing
a high number of commits [3] using 'status = "disabled"' by default. At
least I ask for an explanation regarding this last part before
incrementing the version of this patch. TIA.
[1] fae3a9f023b7 ("ARM: dts: dra7: Add ti,secure-ram node to ocmcram1 node")
[2] 162669876bbe ("ARM: dts: sun9i: Switch to the AC100 RTC clock outputs for
osc32k")
[3] git blame arch/arm64/boot/dts/renesas/r8a7795.dtsi | grep disabled | awk
'{print $1}' | sort -u | wc -l
22
>
> Gr{oetje,eeting}s,
>
> Geert
Thanks,
Eugeniu.
Re: [PATCH v2 2/7] dt-bindings: can: rcar_can: document r8a77965 can support
Hi Kieran, On Fri, Aug 17, 2018 at 05:10:06PM +0100, Kieran Bingham wrote: > Hi Eugeniu, > > On 17/08/18 16:56, Eugeniu Rosca wrote: > > Hello Kieran, > > > > On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote: > >> Hi Eugeniu > >> > >> Thank you for the patch. > >> > >> On 12/08/18 14:31, Eugeniu Rosca wrote: > >>> Document the support for rcar_can on R8A77965 SoC devices. > >>> Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > >>> "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > >> > >> I don't think you needed to say you rewrapped the text in the commit log > >> - but it's fine :) > > > > IMHO "Rewrap text" is pretty much from the same category as "no > > functional change was intended". > > Indeed, but in this instance - there was a functional change. You > modified the paragraph. In fact, mentioning that you have rewrapped the > text, thus implying that you have made no functional change might cause > a reviewer not to look deeper at the actual differences? > > > > As a reviewer, I would take these > > details in the commit description any day (and sometimes I would NAK a > > patch which lacks these details), since they precisely express the goals > > set by the author and make reviewer's life easier. > > > > But, of course, preferences vary and therefore I won't elaborate on that > > too much. > > If this was a separate hunk, which you had re-wrapped without making a > change to - I would absolutely agree with you here. The 'rewrapping' > should be mentioned in the commit message, but this in relation to a > paragraph which you had modified. > > IMO - if you modify a paragraph of text, rewrapping to make sure it fits > the constraints is part of that modification ... but ... yes we are > debating minor details and preferences here ;) - I have no objection to > you mentioning it. Ok, I get your point. I have to tune my auto-pilot to avoid writing down obvious things in the commit descriptions. > > Regards > > Kieran Thanks, Eugeniu.
Re: [PATCH v2 2/7] dt-bindings: can: rcar_can: document r8a77965 can support
Hello Kieran, On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote: > Hi Eugeniu > > Thank you for the patch. > > On 12/08/18 14:31, Eugeniu Rosca wrote: > > Document the support for rcar_can on R8A77965 SoC devices. > > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > > I don't think you needed to say you rewrapped the text in the commit log > - but it's fine :) IMHO "Rewrap text" is pretty much from the same category as "no functional change was intended". As a reviewer, I would take these details in the commit description any day (and sometimes I would NAK a patch which lacks these details), since they precisely express the goals set by the author and make reviewer's life easier. But, of course, preferences vary and therefore I won't elaborate on that too much. > > > Signed-off-by: Eugeniu Rosca > > Reviewed-by: Kieran Bingham > Best regards, Eugeniu.
Re: [PATCH v2 2/7] dt-bindings: can: rcar_can: document r8a77965 can support
Hi Simon, On Fri, Aug 17, 2018 at 11:04:03AM +0200, Simon Horman wrote: > On Sun, Aug 12, 2018 at 03:31:44PM +0200, Eugeniu Rosca wrote: > > Document the support for rcar_can on R8A77965 SoC devices. > > Add R8A77965 to the list of SoCs which require the "assigned-clocks" and > > "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. > > > > Signed-off-by: Eugeniu Rosca > > --- > > Changes in v2: > > - [Kieran Bingham] Simplified commit description. Rewrapped text. > > - [Sergei Shtylyov] Replaced footnotes with inline text. > > - Pushed all dt-bindings patches to the beginning of the series. > > --- > > Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > Reviewed-by: Simon Horman > > As this is a CAN patch I believe it should be: > > * Targeted at the net-next tree. > > * Have net-next in the subject prefix > > e.g.: > > "[PATCH v3 net-next] dt-bindings: can: rcar_can: document r8a77965 > support" I will re-spin this particular patch as v3 with your Reviewed-by and all the suggested names included in the "Sent to" field. > > * Sent to: > Wolfgang Grandegger > Marc Kleine-Budde > "David S. Miller" > linux-...@vger.kernel.org > net...@vger.kernel.org > > As it is a DT binding patch I believe it should also be sent to: > Rob Herring > Mark Rutland > devicet...@vger.kernel.org > > And as it is related to Renesas SoCs it should also be sent to: > Simon Horman > Magnus Damm > linux-renesas-soc@vger.kernel.org > > Sending it to more people is ok too :) Best regards, Eugeniu.
[PATCH v2 7/7] arm64: dts: renesas: r8a77965: m3nulcb-kf: Initial device tree
This is based on the existing KF device tree sources:
$ ls -1 arch/arm64/boot/dts/renesas/*-kf.dts
arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb-kf.dts
arch/arm64/boot/dts/renesas/r8a7795-h3ulcb-kf.dts
arch/arm64/boot/dts/renesas/r8a7796-m3ulcb-kf.dts
Signed-off-by: Eugeniu Rosca
---
Changes in v2:
- [Jacopo Mondi] Removed redundant license text.
- [Simon Horman]
- Renamed DTS 's/r8a77965-ulcb-kf.dts/r8a77965-m3nulcb-kf.dts/'
- Renamed board name 's/M3-N ULCB/M3NULCB/'
- Renamed compatible string 's/renesas,ulcb/renesas,m3nulcb/'
- Adapted Makefile and commit summary line 's/ulcb-kf/m3nulcb-kf/'
- Documented the source of inspiration in the commit description.
---
arch/arm64/boot/dts/renesas/Makefile | 1 +
.../boot/dts/renesas/r8a77965-m3nulcb-kf.dts | 16
2 files changed, 17 insertions(+)
create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-m3nulcb-kf.dts
diff --git a/arch/arm64/boot/dts/renesas/Makefile
b/arch/arm64/boot/dts/renesas/Makefile
index eb158d1f90e9..a8ce6594342d 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -9,6 +9,7 @@ dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-m3ulcb-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_R8A77965) += r8a77965-m3nulcb.dtb
+dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-m3nulcb-kf.dtb
dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb r8a77980-v3hsk.dtb
dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb-kf.dts
b/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb-kf.dts
new file mode 100644
index ..dadad97051b9
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb-kf.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the M3NULCB Kingfisher board
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ * Copyright (C) 2018 Cogent Embedded, Inc.
+ */
+
+#include "r8a77965-m3nulcb.dts"
+#include "ulcb-kf.dtsi"
+
+/ {
+ model = "Renesas M3NULCB Kingfisher board based on r8a77965";
+ compatible = "shimafuji,kingfisher", "renesas,m3nulcb",
+"renesas,r8a77965";
+};
--
2.18.0
[PATCH v2 6/7] arm64: dts: renesas: r8a77965: Add HSCIF0 device node
Fix below DTC error:
Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:36.1-8 Label or path hscif0 not
found
The DTC error occurs *after* the addition of r8a77965-m3nulcb-kf.dts.
Fix it beforehand.
Inspired from v4.12-rc1 commits:
- commit 68cd16107260 ("arm64: dts: r8a7796 dtsi: Add all HSCIF nodes")
- commit 6d50bb893504 ("arm64: dts: r8a7796: Enable HSCIF DMA")
- commit bec0948e810f ("arm64: dts: r8a7796: Add reset control properties")
Signed-off-by: Eugeniu Rosca
---
Changes in v2:
- Slightly improved the commit description.
- Note that this commit could be replaced by
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git/commit/?h=b8e3c8e17611
---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 4da479d3c226..d04a8b671cc2 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -451,6 +451,24 @@
status = "disabled";
};
+ hscif0: serial@e654 {
+ compatible = "renesas,hscif-r8a77965",
+"renesas,rcar-gen3-hscif",
+"renesas,hscif";
+ reg = <0 0xe654 0 0x60>;
+ interrupts = ;
+ clocks = <&cpg CPG_MOD 520>,
+<&cpg CPG_CORE R8A77965_CLK_S3D1>,
+<&scif_clk>;
+ clock-names = "fck", "brg_int", "scif_clk";
+ dmas = <&dmac1 0x31>, <&dmac1 0x30>,
+ <&dmac2 0x31>, <&dmac2 0x30>;
+ dma-names = "tx", "rx", "tx", "rx";
+ power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+ resets = <&cpg 520>;
+ status = "disabled";
+ };
+
hsusb: usb@e659 {
compatible = "renesas,usbhs-r8a7796",
"renesas,rcar-gen3-usbhs";
--
2.18.0
[PATCH v2 5/7] arm64: dts: renesas: r8a77965: Add CAN{0,1} placeholder nodes
According to R-Car Gen3 HW manual rev1.00, R-Car M3-N has two CAN
interfaces, similar to H3, M3-W and other SoCs from the same family.
Add CAN placeholder nodes to avoid below DTC errors:
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
These errors occur *after* the addition of r8a77965-m3nulcb-kf.dts.
Fix them beforehand.
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
---
Changes in v2:
- [Kieran Bingham] Improved commit description:
- Referenced the newer HW manual rev1.00 instead of rev0.55E.
- Kept the "true story" behind the patch. Just made it more clear.
- [Geert Uytterhoeven] Replaced CAN0 and CAN1 nodes with placeholders
(no CAN testing was done to validate the DTS configuration).
---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 486aecacb22a..4da479d3c226 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -656,6 +656,22 @@
status = "disabled";
};
+ can0: can@e6c3 {
+ compatible = "renesas,can-r8a77965",
+"renesas,rcar-gen3-can";
+ reg = <0 0xe6c3 0 0x1000>;
+ /* placeholder */
+ status = "disabled";
+ };
+
+ can1: can@e6c38000 {
+ compatible = "renesas,can-r8a77965",
+"renesas,rcar-gen3-can";
+ reg = <0 0xe6c38000 0 0x1000>;
+ /* placeholder */
+ status = "disabled";
+ };
+
pwm0: pwm@e6e3 {
compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
reg = <0 0xe6e3 0 8>;
--
2.18.0
[PATCH v2 4/7] arm64: dts: renesas: r8a77965: m3nulcb: Initial device tree
Allow the bare M3-N-based ULCB board to boot.
Signed-off-by: Eugeniu Rosca
Reviewed-by: Jacopo Mondi
---
Changes in v2:
- [Jacopo Mondi] Removed redundant license text. Added Reviewed-by.
- [Simon Horman]
- Renamed DTS 's/r8a77965-ulcb.dts/r8a77965-m3nulcb.dts/'
- Renamed board name 's/M3-N ULCB/M3NULCB/'
- Renamed compatible string 's/renesas,ulcb/renesas,m3nulcb/'
- Adapted Makefile and commit summary line 's/ulcb/m3nulcb/'
---
arch/arm64/boot/dts/renesas/Makefile | 1 +
.../boot/dts/renesas/r8a77965-m3nulcb.dts | 33 +++
2 files changed, 34 insertions(+)
create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-m3nulcb.dts
diff --git a/arch/arm64/boot/dts/renesas/Makefile
b/arch/arm64/boot/dts/renesas/Makefile
index 9e2394bc3c62..eb158d1f90e9 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-x.dtb
r8a7796-m3ulcb.dtb
dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-m3ulcb-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_R8A77965) += r8a77965-m3nulcb.dtb
dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb r8a77980-v3hsk.dtb
dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb.dts
b/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb.dts
new file mode 100644
index ..964078b6cc49
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77965-m3nulcb.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the M3NULCB (R-Car Starter Kit Pro) board
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ * Copyright (C) 2018 Cogent Embedded, Inc.
+ */
+
+/dts-v1/;
+#include "r8a77965.dtsi"
+#include "ulcb.dtsi"
+
+/ {
+ model = "Renesas M3NULCB board based on r8a77965";
+ compatible = "renesas,m3nulcb", "renesas,r8a77965";
+
+ memory@4800 {
+ device_type = "memory";
+ /* first 128MB is reserved for secure area. */
+ reg = <0x0 0x4800 0x0 0x7800>;
+ };
+};
+
+&du {
+ clocks = <&cpg CPG_MOD 724>,
+<&cpg CPG_MOD 723>,
+<&cpg CPG_MOD 721>,
+<&versaclock5 1>,
+<&versaclock5 3>,
+<&versaclock5 2>;
+ clock-names = "du.0", "du.1", "du.3",
+ "dclkin.0", "dclkin.1", "dclkin.3";
+};
--
2.18.0
[PATCH v2 3/7] pinctrl: sh-pfc: r8a77965: Add HSCIF0 pins, groups, and functions
According to R-Car Gen3 HW manual Rev.1.00 Apr 2018, M3-N SoC implements
five (0..4) HSCIF channels, similar to H3, M3-W and E3.
The story behind this patch is tackling below dmesg warnings, which pop
up when booting M3NULCB Kingfisher board:
$ dmesg | grep sh-pfc
sh-pfc e606.pin-controller: r8a77965_pfc support registered
sh-pfc e606.pin-controller: function 'hscif0' not supported
sh-pfc e606.pin-controller: invalid function hscif0 in map table
sh-pfc e606.pin-controller: function 'hscif0' not supported
sh-pfc e606.pin-controller: invalid function hscif0 in map table
To fix them, extract the HSCIF0 part from below v4.15-rc1 commits:
- commit 7a362e3488cb ("pinctrl: sh-pfc: r8a7795: Add HSCIF pins, groups, and
functions")
- commit 0e4e4999aac1 ("pinctrl: sh-pfc: r8a7796: Add HSCIF pins, groups, and
functions")
Note that `checkpatch --strict` throws several "CHECK: Please use a
blank line after function/struct/union/enum declarations", which are
ignored for the sake of staying in sync with the aforementioned commits.
Signed-off-by: Eugeniu Rosca
---
Changes in v2:
- Newly added.
- IMHO mirroring the mentioned H3 and M3-W sh-pfc commits *entirely* is
a better option to avoid work fragmentation, but I leave this
decision to the maintainer.
---
drivers/pinctrl/sh-pfc/pfc-r8a77965.c | 32 +++
1 file changed, 32 insertions(+)
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
index d2bbee656381..fe16d194f69b 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77965.c
@@ -1758,6 +1758,28 @@ static const unsigned int du_disp_mux[] = {
DU_DISP_MARK,
};
+/* - HSCIF0 -
*/
+static const unsigned int hscif0_data_pins[] = {
+ /* RX, TX */
+ RCAR_GP_PIN(5, 13), RCAR_GP_PIN(5, 14),
+};
+static const unsigned int hscif0_data_mux[] = {
+ HRX0_MARK, HTX0_MARK,
+};
+static const unsigned int hscif0_clk_pins[] = {
+ /* SCK */
+ RCAR_GP_PIN(5, 12),
+};
+static const unsigned int hscif0_clk_mux[] = {
+ HSCK0_MARK,
+};
+static const unsigned int hscif0_ctrl_pins[] = {
+ /* RTS, CTS */
+ RCAR_GP_PIN(5, 16), RCAR_GP_PIN(5, 15),
+};
+static const unsigned int hscif0_ctrl_mux[] = {
+ HRTS0_N_MARK, HCTS0_N_MARK,
+};
/* - I2C
*/
static const unsigned int i2c1_a_pins[] = {
/* SDA, SCL */
@@ -3169,6 +3191,9 @@ static const struct sh_pfc_pin_group pinmux_groups[] = {
SH_PFC_PIN_GROUP(du_oddf),
SH_PFC_PIN_GROUP(du_cde),
SH_PFC_PIN_GROUP(du_disp),
+ SH_PFC_PIN_GROUP(hscif0_data),
+ SH_PFC_PIN_GROUP(hscif0_clk),
+ SH_PFC_PIN_GROUP(hscif0_ctrl),
SH_PFC_PIN_GROUP(i2c1_a),
SH_PFC_PIN_GROUP(i2c1_b),
SH_PFC_PIN_GROUP(i2c2_a),
@@ -3379,6 +3404,12 @@ static const char * const du_groups[] = {
"du_disp",
};
+static const char * const hscif0_groups[] = {
+ "hscif0_data",
+ "hscif0_clk",
+ "hscif0_ctrl",
+};
+
static const char * const i2c1_groups[] = {
"i2c1_a",
"i2c1_b",
@@ -3651,6 +3682,7 @@ static const char * const usb30_groups[] = {
static const struct sh_pfc_function pinmux_functions[] = {
SH_PFC_FUNCTION(avb),
SH_PFC_FUNCTION(du),
+ SH_PFC_FUNCTION(hscif0),
SH_PFC_FUNCTION(i2c1),
SH_PFC_FUNCTION(i2c2),
SH_PFC_FUNCTION(i2c6),
--
2.18.0
[PATCH v2 1/7] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board
In harmony with ATF and U-Boot outputs [1] and [2], the new board is based on M3-N revision ES1.1 and the amount of memory present on SiP is 2GiB, contiguously addressed. The amount of RAM is mentioned based on the assumption that it is encoded in the board id/string. There is some evidence supporting this in form of last-digit-mismatch between two R-Car H3 ES2.0 ULCB board ids, one with 4GiB and one with 8GiB of RAM (see [3]). [1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21 BL2: PRR is R-Car M3N Ver.1.1 [2] U-Boot 2015.04-00295-* CPU: Renesas Electronics R8A77965 rev 1.1 ---8< DRAM: 1.9 GiB Bank #0: 0x04800 - 0x0bfff, 1.9 GiB ---8< [3] https://patchwork.kernel.org/patch/10555957/#22169325 Signed-off-by: Eugeniu Rosca --- Changes in v2: - [Jacopo Mondi] Emphasized the fact the amount of RAM is encoded in the board id, so documenting it *is* relevant for this commit. - [Simon Horman] - Renamed board name 's/M3-N ULCB/M3NULCB/' - Renamed compatible string 's/renesas,ulcb/renesas,m3nulcb/' - Pushed all dt-bindings patches to the beginning of the series. --- Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt index d8cf740132c6..03a314c7989d 100644 --- a/Documentation/devicetree/bindings/arm/shmobile.txt +++ b/Documentation/devicetree/bindings/arm/shmobile.txt @@ -106,6 +106,8 @@ Boards: compatible = "renesas,lager", "renesas,r8a7790" - M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0)) compatible = "renesas,m3ulcb", "renesas,r8a7796" + - M3NULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1)) +compatible = "renesas,m3nulcb", "renesas,r8a77965" - Marzen (R0P7779A00010S) compatible = "renesas,marzen", "renesas,r8a7779" - Porter (M2-LCDP) -- 2.18.0
[PATCH v2 2/7] dt-bindings: can: rcar_can: document r8a77965 can support
Document the support for rcar_can on R8A77965 SoC devices. Add R8A77965 to the list of SoCs which require the "assigned-clocks" and "assigned-clock-rates" properties (thanks, Sergei). Rewrap text. Signed-off-by: Eugeniu Rosca --- Changes in v2: - [Kieran Bingham] Simplified commit description. Rewrapped text. - [Sergei Shtylyov] Replaced footnotes with inline text. - Pushed all dt-bindings patches to the beginning of the series. --- Documentation/devicetree/bindings/net/can/rcar_can.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt index 94a7f33ac5e9..60daa878c9a2 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,11 +29,10 @@ 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 -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 +Required properties for 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: - assigned-clocks: phandle of clkp2(CANFD) clock. -- 2.18.0
Re: [PATCH 09/14] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board
Hi again Jacopo, On Mon, Aug 06, 2018 at 12:40:02AM +0200, Eugeniu Rosca wrote: > Hi Jacopo, > > Thanks for your comments. Please, find my replies below. > > On Sun, Aug 05, 2018 at 10:15:43AM +0200, jacopo mondi wrote: > > Helle Eugeniu, > > > > On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote: > > > In harmony with ATF and U-Boot outputs [1] and [2], the new board is > > > based on M3-N revision ES1.1 and the amount of memory present on SiP > > > is 2GiB, contiguously addressed. > > > > Not sure why the amount of installed system memory is relevant for > > this commit.. > > To be honest, I don't know precisely what's encoded in the board string > (particularly the one documented in this commit RTP0RC77965SKBX010SA00). > > The only thing unmistakenly present there is the SoC model 77965 (i.e. > M3-N), but I am quite clueless about the rest. > > What I can say for sure is that the end-user experience of a R-Car Gen3 > reference board clearly depends on below parameters: > - [A] SoC (model, revision) including SRAM and on-chip peripherals > - [B] DRAM (amount, linear/2ch/4ch split) > - [C] Hyperflash (amount) > - [D] board's PCB (revision) > - [E] board's off-chip peripherals (model, revision) > > I don't know how many of these parameters are embedded in the board > string, but since you are suggesting that RAM amount is not (IOW > Renesas will potentially release several RTP0RC77965SKBX010SA00 > boards with different amounts of memory), I will happily update > the commit description. Since I found two H3-ES2.0 Starter Kit samples with different amount of RAM (4 vs 8 GiB) each having a unique board id [1], I take this as evidence that Renesas encodes the amount of RAM in the board id. Therefore, I will not change the description of this patch in v2, unless you comment/NAK. Thank you. [1] https://patchwork.kernel.org/patch/10555957/#22169325 Best regards, Eugeniu.
Re: [PATCH 01/14] arm64: dts: renesas: Cut redundant in *-ulcb*.dts
Hi Simon, Laurent, On Fri, Aug 10, 2018 at 01:22:49PM +0200, Simon Horman wrote: > On Mon, Aug 06, 2018 at 01:35:08PM +0300, Laurent Pinchart wrote: [snip] > > 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. > > At this point I'd prefer to keep the current, albeit imperfect scheme, > and avoid churn. To be able to push a v2, I interpret this input as using below strings for the newly supported M3-N Starter Kit board: - r8a77965-m3nulcb.dts and r8a77965-m3nulcb-kf.dts for device tree sources - "renesas,m3nulcb" for compatible string - M3NULCB for board name in the shmobile.txt DT bindings I still think there is some redundancy in this naming scheme, but I can live with that. Thanks, Eugeniu.
Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
Hello Geert, Laurent, Morimoto-san, On Tue, Aug 07, 2018 at 10:30:14AM +0200, Geert Uytterhoeven wrote: > Hi Laurent, > > On Tue, Aug 7, 2018 at 10:21 AM Laurent Pinchart > wrote: > > On Tuesday, 7 August 2018 11:18:11 EEST Kuninori Morimoto wrote: > > > > 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") > > > > > > "renesas,ulcb" is very generic naming. > > > Not only h3/m3, if we had v3/e3/d3 etc ulcb, > > > > Furthermore, "ulcb" is an unofficial term, the boards are named "starter > > kit" > > (SK). Using internal names in code or device tree sources is a normal > > practice > > and is fine with me, but I'm a bit bothered by the fact that the H3/M3 > > boards > > are called ULCB in DT, while the V3 board are called SK. I wonder if we > > should > > unify that or if it's too late. > > Perhaps we should. > > Renesas has a long history of boards named SK or RSK. > The inconsistency started when suddenly SK was spelled out in full, with > "Premier" or "Pro" added to differentiate, and the need arose for a shorter > nickname, which became "ULCB" I really appreciate your comments, but it looks like at least the following open questions prevent this series to advance into v2 (feel free to point out flaws in my understanding): - [A] it is not clear if H3ULCB, M3ULCB and M3NULCB boards should use a common compatible string or dedicated ones. - [B] In case a common string is used for all *ULCB boards, should it drop the unofficial "ulcb" (Ultra Low Cost Board, thanks Laurent) in exchange to "sk", "starter-kit" or similar? - [C] Same as [A] and [B], but applied to ULCB DTS filenames, which are currently formed based on the same "ulcb" stem. IMHO these questions go somewhat beyond the scope of M3-N ULCB bring-up. In spite of this, I would be happy to implement your proposals. I am also fine to wait a couple more days to collect more feedback, as well as let the ideas/thoughts to settle. However, if you expect the latter to take longer, maybe we can find some "acceptable" solution and defer the naming issues to a later point? Thanks, Eugeniu.
Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
On Sun, Aug 05, 2018 at 01:11:02AM +0200, Eugeniu Rosca wrote: > 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)) FWIW/FTR, I have found the schematics of RTP0RC7795SKB00010S (looks like a H3-ES1x Starter-Kit) and this specific board doesn't have an entry in Documentation/devicetree/bindings/arm/shmobile.txt. Also, there is RTP0RC77951SKBX010SA03 (8GB version H3-ES20 Starter-Kit). This is also not documented. Interestingly, there is one digit difference between: - 4GiB H3-ES2.0 Starter-Kit: RTP0RC77951SKBX010SA00 - 8GiB H3-ES2.0 Starter-Kit: RTP0RC77951SKBX010SA03 which means Renesas encodes the amount of RAM in the board id/string. > -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) > -- > 2.18.0 Best regards, Eugeniu.
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 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 CPG_MOD 916>,
> > + <&cpg CPG_CORE R8A77965_CLK_CANFD>,
> > + <&can_clk>;
> > + clock-names = "clkp1", "clkp2", "can_clk";
> > + assigned-clocks = <&cpg 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 = <&sysc R8A77965_PD_ALWAYS_ON>;
> > + resets = <&cpg 916>;
> > + status = "disabled";
> > + };
> > +
> > + can1: can@e6c38000 {
> > + compatibl
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
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.
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 13/14] arm64: dts: renesas: r8a77965: Add HSCIF0 device node
Hello Simon, Geert, Takeshi,
On Sun, Aug 05, 2018 at 01:11:13AM +0200, Eugeniu Rosca wrote:
> Add hscif0 node to avoid below r8a77965-ulcb-kf.dtb build failure:
> Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:36.1-8 Label or path hscif0
> not found
>
> Inspired from v4.12-rc1 commits:
> - commit 68cd16107260 ("arm64: dts: r8a7796 dtsi: Add all HSCIF nodes")
> - commit 6d50bb893504 ("arm64: dts: r8a7796: Enable HSCIF DMA")
> - commit bec0948e810f ("arm64: dts: r8a7796: Add reset control properties")
>
> Signed-off-by: Eugeniu Rosca
> ---
> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 18 ++
> 1 file changed, 18 insertions(+)
I noticed that this patch (adding HSCIF0 node only) conflicts with
below commit (adding all HSCIF nodes) from the "devel" branch of
horms/renesas.git repository:
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git/commit/?h=b8e3c8e17611
Please, feel free to ignore my patch. It only ensures that this series
doesn't break the "make dtbs" build.
Thanks,
Eugeniu.
Re: [PATCH 14/14] arm64: dts: renesas: r8a77965: ulcb-kf: Initial device tree
Hi Jacopo,
On Sun, Aug 05, 2018 at 11:01:36AM +0200, jacopo mondi wrote:
> Hi Eugeniu,
>
> On Sun, Aug 05, 2018 at 01:11:14AM +0200, Eugeniu Rosca wrote:
> > Signed-off-by: Eugeniu Rosca
> > ---
> > arch/arm64/boot/dts/renesas/Makefile | 1 +
> > arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts | 20
> > 2 files changed, 21 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/Makefile
> > b/arch/arm64/boot/dts/renesas/Makefile
> > index ad7be9a8ca56..3e2c5be65a14 100644
> > --- a/arch/arm64/boot/dts/renesas/Makefile
> > +++ b/arch/arm64/boot/dts/renesas/Makefile
> > @@ -9,6 +9,7 @@ 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_R8A77965) += r8a77965-ulcb.dtb
> > +dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-ulcb-kf.dtb
> > dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
> > dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb r8a77980-v3hsk.dtb
> > dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
> > b/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
> > new file mode 100644
> > index ..5c719fb87d8b
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
> > @@ -0,0 +1,20 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Device Tree Source for the M3-N ULCB Kingfisher board
> > + *
> > + * Copyright (C) 2018 Renesas Electronics Corp.
> > + * Copyright (C) 2018 Cogent Embedded, Inc.
> > + *
> > + * This file is licensed under the terms of the GNU General Public License
> > + * version 2. This program is licensed "as is" without any warranty of any
> > + * kind, whether express or implied.
> > + */
>
> Drop the license text please :)
I will. Thanks!
>
> Thanks
> j
> > +
> > +#include "r8a77965-ulcb.dts"
> > +#include "ulcb-kf.dtsi"
> > +
> > +/ {
> > + model = "Renesas ULCB Kingfisher board based on r8a77965";
> > + compatible = "shimafuji,kingfisher", "renesas,ulcb",
> > +"renesas,r8a77965";
> > +};
> > --
> > 2.18.0
> >
Best regards,
Eugeniu.
Re: [PATCH 09/14] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board
Hi Jacopo,
Thanks for your comments. Please, find my replies below.
On Sun, Aug 05, 2018 at 10:15:43AM +0200, jacopo mondi wrote:
> Helle Eugeniu,
>
> On Sun, Aug 05, 2018 at 01:11:09AM +0200, Eugeniu Rosca wrote:
> > In harmony with ATF and U-Boot outputs [1] and [2], the new board is
> > based on M3-N revision ES1.1 and the amount of memory present on SiP
> > is 2GiB, contiguously addressed.
>
> Not sure why the amount of installed system memory is relevant for
> this commit..
To be honest, I don't know precisely what's encoded in the board string
(particularly the one documented in this commit RTP0RC77965SKBX010SA00).
The only thing unmistakenly present there is the SoC model 77965 (i.e.
M3-N), but I am quite clueless about the rest.
What I can say for sure is that the end-user experience of a R-Car Gen3
reference board clearly depends on below parameters:
- [A] SoC (model, revision) including SRAM and on-chip peripherals
- [B] DRAM (amount, linear/2ch/4ch split)
- [C] Hyperflash (amount)
- [D] board's PCB (revision)
- [E] board's off-chip peripherals (model, revision)
I don't know how many of these parameters are embedded in the board
string, but since you are suggesting that RAM amount is not (IOW
Renesas will potentially release several RTP0RC77965SKBX010SA00
boards with different amounts of memory), I will happily update
the commit description.
>
> >
> > [1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21
> > BL2: PRR is R-Car M3N Ver.1.1
> >
> > [2] U-Boot 2015.04-00295-*
> > CPU: Renesas Electronics R8A77965 rev 1.1
> > ---8<
> > DRAM: 1.9 GiB
> > Bank #0: 0x04800 - 0x0bfff, 1.9 GiB
> > ---8<
> >
> > Signed-off-by: Eugeniu Rosca
> > ---
> > Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt
> > b/Documentation/devicetree/bindings/arm/shmobile.txt
> > index f391dba10574..2f3494a0107c 100644
> > --- a/Documentation/devicetree/bindings/arm/shmobile.txt
> > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt
> > @@ -106,6 +106,8 @@ Boards:
> > compatible = "renesas,lager", "renesas,r8a7790"
> >- M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0))
> > compatible = "renesas,ulcb", "renesas,r8a7796"
> > + - M3-N ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1))
>
> Other documented ULCB description entries in this file are
> H3ULCB and M3ULCB, so for consistency you should add M3NULCB, which
> isn't that nice.
This was my initial intent, but then I noticed commits like
519df0e05d27 ("dt-bindings: arm: consistently name r8a77965 as M3-N"),
which try to use "M3-N" notation consistently and avoid "M3N". So, I
tried to avoid "M3N" too.
>
> Imo, or you either replace "[H|M]3ULCB" with "[H|M]3 ULCB" in other
> entries and you keep your "M3-N ULCB" here, which is nicer (you could
> do that in patch 2).
>
> Or maybe could you consider doing what has been done for
> Salvator-x(s), which do not have the SoC model name in the entry description
> at all (but please wait for others to comment before doing something
> like that):
I prefer this second variant, but I will wait for others to comment.
>
> - Salvator-X (RTP0RC7795SIPB0010S)
> compatible = "renesas,salvator-x", "renesas,r8a7795"
> - Salvator-X (RTP0RC7796SIPB0011S)
> compatible = "renesas,salvator-x", "renesas,r8a7796"
> - Salvator-X (RTP0RC7796SIPB0011S (M3-N))
> compatible = "renesas,salvator-x", "renesas,r8a77965"
> - Salvator-XS (Salvator-X 2nd version, RTP0RC7795SIPB0012S)
> compatible = "renesas,salvator-xs", "renesas,r8a7795"
> - Salvator-XS (Salvator-X 2nd version, RTP0RC7796SIPB0012S)
> compatible = "renesas,salvator-xs", "renesas,r8a7796"
> - Salvator-XS (Salvator-X 2nd version, RTP0RC77965SIPB012S)
> compatible = "renesas,salvator-xs", "renesas,r8a77965"
>
> This would then be
> - ULCB (R-Car Starter Kit Premier, RTP0RC7795SKBX0010SA00 (H3 ES1.1))
> ULCB (R-Car Starter Kit Premier, RTP0RC77951SKBX010SA00 (H3 ES2.0))
> compatible = "renesas,ulcb", "renesas,r8a7795
> - ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0)
> compatible = "renesas,ulcb", "rene
[PATCH 08/14] arm64: dts: renesas: r8a7796: ulcb-kf: Use "renesas,ulcb" compatible
Following the recent change in dt-bindings [1], switch from
"renesas,m3ulcb" to "renesas,ulcb" compatible string.
[1] Documentation/devicetree/bindings/arm/shmobile.txt
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a7796-ulcb-kf.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-ulcb-kf.dts
b/arch/arm64/boot/dts/renesas/r8a7796-ulcb-kf.dts
index faa32c28eef7..5bd655c0355e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-ulcb-kf.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-ulcb-kf.dts
@@ -14,6 +14,6 @@
/ {
model = "Renesas M3ULCB Kingfisher board based on r8a7796";
- compatible = "shimafuji,kingfisher", "renesas,m3ulcb",
+ compatible = "shimafuji,kingfisher", "renesas,ulcb",
"renesas,r8a7796";
};
--
2.18.0
[PATCH 12/14] arm64: dts: renesas: r8a77965: add CAN support
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
---
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 CPG_MOD 916>,
+ <&cpg CPG_CORE R8A77965_CLK_CANFD>,
+ <&can_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = <&cpg CPG_CORE R8A77965_CLK_CANFD>;
+ assigned-clock-rates = <4000>;
+ power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+ resets = <&cpg 916>;
+ status = "disabled";
+ };
+
+ can1: can@e6c38000 {
+ compatible = "renesas,can-r8a77965",
+"renesas,rcar-gen3-can";
+ reg = <0 0xe6c38000 0 0x1000>;
+ interrupts = ;
+ clocks = <&cpg CPG_MOD 915>,
+ <&cpg CPG_CORE R8A77965_CLK_CANFD>,
+ <&can_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = <&cpg CPG_CORE R8A77965_CLK_CANFD>;
+ assigned-clock-rates = <4000>;
+ power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+ resets = <&cpg 915>;
+ status = "disabled";
+ };
+
pwm0: pwm@e6e3 {
compatible = "renesas,pwm-r8a77965", "renesas,pwm-rcar";
reg = <0 0xe6e3 0 8>;
--
2.18.0
[PATCH 07/14] arm64: dts: renesas: r8a7796: ulcb: Use "renesas,ulcb" compatible
Following the recent change in dt-bindings [1], switch from
"renesas,m3ulcb" to "renesas,ulcb" compatible string.
[1] Documentation/devicetree/bindings/arm/shmobile.txt
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a7796-ulcb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-ulcb.dts
b/arch/arm64/boot/dts/renesas/r8a7796-ulcb.dts
index daee1f1a3f68..c7f5ddd70a5c 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-ulcb.dts
@@ -15,7 +15,7 @@
/ {
model = "Renesas M3ULCB board based on r8a7796";
- compatible = "renesas,m3ulcb", "renesas,r8a7796";
+ compatible = "renesas,ulcb", "renesas,r8a7796";
memory@4800 {
device_type = "memory";
--
2.18.0
[PATCH 10/14] arm64: dts: renesas: r8a77965: ulcb: Initial device tree
Allow the bare M3-N-based ULCB board to boot.
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/Makefile | 1 +
arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts | 37
+
2 files changed, 38 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/Makefile
b/arch/arm64/boot/dts/renesas/Makefile
index 5debb02fad2c..ad7be9a8ca56 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -8,6 +8,7 @@ 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_R8A77965) += r8a77965-ulcb.dtb
dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb r8a77980-v3hsk.dtb
dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts
b/arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts
new file mode 100644
index ..4dde14738fb0
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the M3-N ULCB (R-Car Starter Kit Pro) board
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ * Copyright (C) 2018 Cogent Embedded, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+/dts-v1/;
+#include "r8a77965.dtsi"
+#include "ulcb.dtsi"
+
+/ {
+ model = "Renesas ULCB board based on r8a77965";
+ compatible = "renesas,ulcb", "renesas,r8a77965";
+
+ memory@4800 {
+ device_type = "memory";
+ /* first 128MB is reserved for secure area. */
+ reg = <0x0 0x4800 0x0 0x7800>;
+ };
+};
+
+&du {
+ clocks = <&cpg CPG_MOD 724>,
+<&cpg CPG_MOD 723>,
+<&cpg CPG_MOD 721>,
+<&versaclock5 1>,
+<&versaclock5 3>,
+<&versaclock5 2>;
+ clock-names = "du.0", "du.1", "du.3",
+ "dclkin.0", "dclkin.1", "dclkin.3";
+};
--
2.18.0
[PATCH 13/14] arm64: dts: renesas: r8a77965: Add HSCIF0 device node
Add hscif0 node to avoid below r8a77965-ulcb-kf.dtb build failure:
Error: arch/arm64/boot/dts/renesas/ulcb-kf.dtsi:36.1-8 Label or path hscif0 not
found
Inspired from v4.12-rc1 commits:
- commit 68cd16107260 ("arm64: dts: r8a7796 dtsi: Add all HSCIF nodes")
- commit 6d50bb893504 ("arm64: dts: r8a7796: Enable HSCIF DMA")
- commit bec0948e810f ("arm64: dts: r8a7796: Add reset control properties")
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index cb8f8573d9ef..53405af41e34 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -451,6 +451,24 @@
status = "disabled";
};
+ hscif0: serial@e654 {
+ compatible = "renesas,hscif-r8a77965",
+"renesas,rcar-gen3-hscif",
+"renesas,hscif";
+ reg = <0 0xe654 0 0x60>;
+ interrupts = ;
+ clocks = <&cpg CPG_MOD 520>,
+<&cpg CPG_CORE R8A77965_CLK_S3D1>,
+<&scif_clk>;
+ clock-names = "fck", "brg_int", "scif_clk";
+ dmas = <&dmac1 0x31>, <&dmac1 0x30>,
+ <&dmac2 0x31>, <&dmac2 0x30>;
+ dma-names = "tx", "rx", "tx", "rx";
+ power-domains = <&sysc R8A77965_PD_ALWAYS_ON>;
+ resets = <&cpg 520>;
+ status = "disabled";
+ };
+
hsusb: usb@e659 {
compatible = "renesas,usbhs-r8a7796",
"renesas,rcar-gen3-usbhs";
--
2.18.0
[PATCH 14/14] arm64: dts: renesas: r8a77965: ulcb-kf: Initial device tree
Signed-off-by: Eugeniu Rosca
---
arch/arm64/boot/dts/renesas/Makefile | 1 +
arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts | 20
2 files changed, 21 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/Makefile
b/arch/arm64/boot/dts/renesas/Makefile
index ad7be9a8ca56..3e2c5be65a14 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -9,6 +9,7 @@ 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_R8A77965) += r8a77965-ulcb.dtb
+dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-ulcb-kf.dtb
dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb
dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb r8a77980-v3hsk.dtb
dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
b/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
new file mode 100644
index ..5c719fb87d8b
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree Source for the M3-N ULCB Kingfisher board
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ * Copyright (C) 2018 Cogent Embedded, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include "r8a77965-ulcb.dts"
+#include "ulcb-kf.dtsi"
+
+/ {
+ model = "Renesas ULCB Kingfisher board based on r8a77965";
+ compatible = "shimafuji,kingfisher", "renesas,ulcb",
+"renesas,r8a77965";
+};
--
2.18.0
[PATCH 11/14] dt-bindings: can: rcar_can: document r8a77965 can support
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 @@ -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 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) -- 2.18.0
[PATCH 09/14] dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board
In harmony with ATF and U-Boot outputs [1] and [2], the new board is based on M3-N revision ES1.1 and the amount of memory present on SiP is 2GiB, contiguously addressed. [1] BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21 BL2: PRR is R-Car M3N Ver.1.1 [2] U-Boot 2015.04-00295-* CPU: Renesas Electronics R8A77965 rev 1.1 ---8< DRAM: 1.9 GiB Bank #0: 0x04800 - 0x0bfff, 1.9 GiB ---8< Signed-off-by: Eugeniu Rosca --- Documentation/devicetree/bindings/arm/shmobile.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt index f391dba10574..2f3494a0107c 100644 --- a/Documentation/devicetree/bindings/arm/shmobile.txt +++ b/Documentation/devicetree/bindings/arm/shmobile.txt @@ -106,6 +106,8 @@ Boards: compatible = "renesas,lager", "renesas,r8a7790" - M3ULCB (R-Car Starter Kit Pro, RTP0RC7796SKBX0010SA09 (M3 ES1.0)) compatible = "renesas,ulcb", "renesas,r8a7796" + - M3-N ULCB (R-Car Starter Kit Pro, RTP0RC77965SKBX010SA00 (M3-N ES1.1)) +compatible = "renesas,ulcb", "renesas,r8a77965" - Marzen (R0P7779A00010S) compatible = "renesas,marzen", "renesas,r8a7779" - Porter (M2-LCDP) -- 2.18.0
[PATCH 00/14] Add minimal DTS support for M3-N Starter Kit
Hello Renesas community,
This patch series arised during the bring-up of M3-N-based ULCB board.
It adds minimal DTS support and documents some new DT bindings.
It also does some collateral changes in the area of ULCB compatible
strings, to avoid defining something like "renesas,m3nulcb". Similar to
how "renesas,salvator-x{s}" compatibles are constructed, in my opinion,
"renesas,(m3|m3n|h3)ulcb" should not use the SoC model in their name,
for consistency.
Another change done in preparation of the M3-N DTS commits is renaming
the ULCB device trees:
- from "-ulcb*.dts"
- to "-ulcb*.dts"
These patches have been tested on M3-N ULCB using v4.18-rc7 kernel.
Any comments would be greatly appreciated.
Best regards,
Eugeniu.
Eugeniu Rosca (14):
arm64: dts: renesas: Cut redundant in *-ulcb*.dts
dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
arm64: dts: renesas: r8a7795-es1: ulcb: Use "renesas,ulcb" compatible
arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible
arm64: dts: renesas: r8a7795: ulcb: Use "renesas,ulcb" compatible
arm64: dts: renesas: r8a7795: ulcb-kf: Use "renesas,ulcb" compatible
arm64: dts: renesas: r8a7796: ulcb: Use "renesas,ulcb" compatible
arm64: dts: renesas: r8a7796: ulcb-kf: Use "renesas,ulcb" compatible
dt-bindings: arm: Document Renesas R-Car M3-N-based ULCB board
arm64: dts: renesas: r8a77965: ulcb: Initial device tree
dt-bindings: can: rcar_can: document r8a77965 can support
arm64: dts: renesas: r8a77965: add CAN support
arm64: dts: renesas: r8a77965: Add HSCIF0 device node
arm64: dts: renesas: r8a77965: ulcb-kf: Initial device tree
.../devicetree/bindings/arm/shmobile.txt | 6 ++-
.../devicetree/bindings/net/can/rcar_can.txt | 10 ++--
arch/arm64/boot/dts/renesas/Makefile | 14 +++---
...-h3ulcb-kf.dts => r8a7795-es1-ulcb-kf.dts} | 4 +-
...95-es1-h3ulcb.dts => r8a7795-es1-ulcb.dts} | 2 +-
...7795-h3ulcb-kf.dts => r8a7795-ulcb-kf.dts} | 4 +-
.../{r8a7795-h3ulcb.dts => r8a7795-ulcb.dts} | 2 +-
...7796-m3ulcb-kf.dts => r8a7796-ulcb-kf.dts} | 4 +-
.../{r8a7796-m3ulcb.dts => r8a7796-ulcb.dts} | 2 +-
.../boot/dts/renesas/r8a77965-ulcb-kf.dts | 20
arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts | 37 ++
arch/arm64/boot/dts/renesas/r8a77965.dtsi | 50 +++
12 files changed, 135 insertions(+), 20 deletions(-)
rename arch/arm64/boot/dts/renesas/{r8a7795-es1-h3ulcb-kf.dts =>
r8a7795-es1-ulcb-kf.dts} (84%)
rename arch/arm64/boot/dts/renesas/{r8a7795-es1-h3ulcb.dts =>
r8a7795-es1-ulcb.dts} (94%)
rename arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb-kf.dts =>
r8a7795-ulcb-kf.dts} (84%)
rename arch/arm64/boot/dts/renesas/{r8a7795-h3ulcb.dts => r8a7795-ulcb.dts}
(96%)
rename arch/arm64/boot/dts/renesas/{r8a7796-m3ulcb-kf.dts =>
r8a7796-ulcb-kf.dts} (84%)
rename arch/arm64/boot/dts/renesas/{r8a7796-m3ulcb.dts => r8a7796-ulcb.dts}
(95%)
create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-ulcb-kf.dts
create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-ulcb.dts
--
2.18.0
[PATCH 04/14] arm64: dts: renesas: r8a7795-es1: ulcb-kf: Use "renesas,ulcb" compatible
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";
};
--
2.18.0
[PATCH 05/14] arm64: dts: renesas: r8a7795: ulcb: Use "renesas,ulcb" compatible
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-ulcb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-ulcb.dts
b/arch/arm64/boot/dts/renesas/r8a7795-ulcb.dts
index 0afe777973de..9cccdef23634 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-ulcb.dts
@@ -15,7 +15,7 @@
/ {
model = "Renesas H3ULCB board based on r8a7795 ES2.0+";
- compatible = "renesas,h3ulcb", "renesas,r8a7795";
+ compatible = "renesas,ulcb", "renesas,r8a7795";
memory@4800 {
device_type = "memory";
--
2.18.0
[PATCH 03/14] arm64: dts: renesas: r8a7795-es1: ulcb: Use "renesas,ulcb" compatible
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.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
index dd4f9b6a4254..8ec1695ca9ee 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
@@ -15,7 +15,7 @@
/ {
model = "Renesas H3ULCB board based on r8a7795 ES1.x";
- compatible = "renesas,h3ulcb", "renesas,r8a7795";
+ compatible = "renesas,ulcb", "renesas,r8a7795";
memory@4800 {
device_type = "memory";
--
2.18.0
[PATCH 06/14] arm64: dts: renesas: r8a7795: ulcb-kf: Use "renesas,ulcb" compatible
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-ulcb-kf.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
b/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
index 70a0c5332d54..5a3a79bd0849 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
@@ -14,6 +14,6 @@
/ {
model = "Renesas H3ULCB Kingfisher board based on r8a7795 ES2.0+";
- compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
+ compatible = "shimafuji,kingfisher", "renesas,ulcb",
"renesas,r8a7795";
};
--
2.18.0
[PATCH 01/14] arm64: dts: renesas: Cut redundant in *-ulcb*.dts
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
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
b/arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
similarity index 100%
rename from arch/arm64/boot/dts/renesas/r8a7795-es1-h3ulcb.dts
rename to arch/arm64/boot/dts/renesas/r8a7795-es1-ulcb.dts
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb-kf.dts
b/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
similarity index 94%
rename from arch/arm64/boot/dts/renesas/r8a7795-h3ulcb-kf.dts
rename to arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
index 4403227c0f97..70a0c5332d54 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb-kf.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-ulcb-kf.dts
@@ -9,7 +9,7 @@
* kind, whether express or implied.
*/
-#include "r8a7795-h3ulcb.dts"
+#include "r8a7795-ulcb.dts"
#include "ulcb-kf.dtsi"
/ {
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
b/arch/arm64/boot/dts/renesas/r8a7795-ulcb.dts
similarity index 100%
rename from arch/arm64/boot/dts/renesas/r8a
[PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
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" 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) -- 2.18.0
Re: [PATCH] i2c: rcar: initialize earlier using subsys_initcall()
[re-sending due to lost To: field] Hello Wolfram, Vladimir, Thanks for your precious inputs. I think you outlined two topics in your comments (based on the description submitted with the patch). One (primary?) is related to async probing and one (secondary, but still interesting) is related to the minor (~7ms -> ~1ms) startup improvement of the rcar-i2c driver. I did a little bit of research to understand what's behind the startup improvement (hoping it is simpler to explain) and I think I reached some conclusions. I used vanilla v4.16 kernel/defconfig/dtb and the h3-es20-salvator-X target. The only configuration change was to enable CONFIG_DEBUG_DRIVER and to increase CONFIG_LOG_BUF_SHIFT from 17 to 22 to avoid overwriting the printk buffer. Kernel was boot-ed "quiet"-ly. With this HW/SW setup, I am able to confirm (based on 5 dmesg logs with and w/o the submitted patch) that: [1] W/o the submitted patch, rcar_i2c_driver_init takes around 7092 us [2] With the submitted patch, rcar_i2c_driver_init takes around 1241 us Comparing the marked below in the two cases: calling rcar_i2c_driver_init initcall rcar_i2c_driver_init+0x0/0x20 returned 0 after X us I notice that w/o the proposed patch, is consistently more rich, containing output related to additional three drivers: * cs2000-cp * rcar-dmac * ohci-platform These additional lines cumulatively consume around 5-6 ms, which is exactly the difference between [1] and [2] seen above. To explain why these lines are present in [1], but not in [2], I checked the order of initialization of the mentioned drivers. Here is what I see: [11] W/o the submitted patch (in the order of execution): InitcallTime (us) cs2000_driver_init 30 rcar_dmac_driver_init8107 ohci_platform_init 178199 rcar_i2c_driver_init 7092 SUM 193428 [22] With the submitted patch (in the order of execution): InitcallTime (us) rcar_i2c_driver_init 1241 cs2000_driver_init 5667 rcar_dmac_driver_init8160 ohci_platform_init 178110 SUM193178 So, the idea is that the startup improvement of rcar_i2c_driver_init() comes at the cost of a slower cs2000_driver_init(). In the end, there is no benefit and this doesn't work as an argument why the patch should go upstream. With regard to the primary argument, that the proposed patch reduces/minimizes the boot time variance when switching certain drivers like rcar-du from synchronous to asynchronous probing, I can clearly sense this in my measurements, but I'm afraid I won't be able to provide a simple and comprehensive explanation why this happens, since we enter the territory of concurrent/parallel driver initialization. If evidence in the form of measurements showing improved behavior is not enough for the patch to be accepted, then we have no choice but to keep the patch out-of-tree as a "workaround", hoping that we'll be able to come up with a better/cleaner solution in future. If you ever stumble upon a mailing list discussing how to "stabilize" the boot time as a consequence of using asynchronous probing, then please drop a link and this will be appreciated. Many thanks, Eugeniu.
Re: [PATCH] i2c: rcar: initialize earlier using subsys_initcall()
Hello Wolfram, Vladimir, Thanks for your precious inputs. I think you outlined two topics in your comments (based on the description submitted with the patch). One (primary?) is related to async probing and one (secondary, but still interesting) is related to the minor (~7ms -> ~1ms) startup improvement of the rcar-i2c driver. I did a little bit of research to understand what's behind the startup improvement (hoping it is simpler to explain) and I think I reached some conclusions. I used vanilla v4.16 kernel/defconfig/dtb and the h3-es20-salvator-X target. The only configuration change was to enable CONFIG_DEBUG_DRIVER and to increase CONFIG_LOG_BUF_SHIFT from 17 to 22 to avoid overwriting the printk buffer. Kernel was boot-ed "quiet"-ly. With this HW/SW setup, I am able to confirm (based on 5 dmesg logs with and w/o the submitted patch) that: [1] W/o the submitted patch, rcar_i2c_driver_init takes around 7092 us [2] With the submitted patch, rcar_i2c_driver_init takes around 1241 us Comparing the marked below in the two cases: calling rcar_i2c_driver_init initcall rcar_i2c_driver_init+0x0/0x20 returned 0 after X us I notice that w/o the proposed patch, is consistently more rich, containing output related to additional three drivers: * cs2000-cp * rcar-dmac * ohci-platform These additional lines cumulatively consume around 5-6 ms, which is exactly the difference between [1] and [2] seen above. To explain why these lines are present in [1], but not in [2], I checked the order of initialization of the mentioned drivers. Here is what I see: [11] W/o the submitted patch (in the order of execution): InitcallTime (us) cs2000_driver_init 30 rcar_dmac_driver_init8107 ohci_platform_init 178199 rcar_i2c_driver_init 7092 SUM 193428 [22] With the submitted patch (in the order of execution): InitcallTime (us) rcar_i2c_driver_init 1241 cs2000_driver_init 5667 rcar_dmac_driver_init8160 ohci_platform_init 178110 SUM193178 So, the idea is that the startup improvement of rcar_i2c_driver_init() comes at the cost of a slower cs2000_driver_init(). In the end, there is no benefit and this doesn't work as an argument why the patch should go upstream. With regard to the primary argument, that the proposed patch reduces/minimizes the boot time variance when switching certain drivers like rcar-du from synchronous to asynchronous probing, I can clearly sense this in my measurements, but I'm afraid I won't be able to provide a simple and comprehensive explanation why this happens, since we enter the territory of concurrent/parallel driver initialization. If evidence in the form of measurements showing improved behavior is not enough for the patch to be accepted, then we have no choice but to keep the patch out-of-tree as a "workaround", hoping that we'll be able to come up with a better/cleaner solution in future. If you ever stumble upon a mailing list discussing how to "stabilize" the boot time as a consequence of using asynchronous probing, then please drop a link and this will be appreciated. Many thanks, Eugeniu.
Re: [PATCH] i2c: rcar: initialize earlier using subsys_initcall()
Hello Wolfram and i2c/Renesas contributors,
To give you some background behind this change, it is done in the
context of rcar3 boot optimization, while trying to put our past
experience in practice for reaching the "rvc-in-less-than-2s"
requirement for our customers.
The current commit description should already speak by itself. While
switching drivers to async probing is hardly suitable with mainline
(am I wrong?), I believe this small change should be harmless. If
there are any concerns, we will do our best to properly handle them.
Special thanks to Vladimir, for reviewing and submitting the patch.
Best regards,
Eugeniu.
On Fri, Mar 16, 2018 at 12:58:52PM +0200, Vladimir Zapolskiy wrote:
> From: Eugeniu Rosca
>
> The purpose of this patch looks pretty similar to:
> 104522806a7d ("i2c: designware: dw_i2c_init_driver as subsys initcall")
> 74f56c4ad4e4 ("i2c-bfin-twi: move setup to the earlier subsys initcall")
> b8680784875b ("i2c-gpio: Move initialization code to subsys_initcall()")
> 5d3f33318a6c ("[PATCH] i2c-imx: make bus available early")
> ccb3bc16b489 ("i2c-sh_mobile: change module_init() to subsys_initcall()")
> 18dc83a6ea48 ("i2c: i2c-s3c2410: Initialise Samsung I2C controller early")
> 2514cca06be9 ("[ARM] 5394/1: Add static bus numbering support to
> i2c-versatile")
> 47a9b1379a5e ("i2c-pxa: Initialize early")
>
> However, the story behind it might be a bit different.
> Experimenting with async probing in various rcar-specific drivers (e.g.
> rcar-du, vsp1, rcar-fcp, rcar-vin), it was noticed that in most of
> the cases enabling async probing in one driver introduced some degree of
> inconsistency in the initialization of other builtin drivers.
>
> To give an example, with pure sequential driver initialization, based
> on 5 dmesg logs, the builtin kernel initialization deviation is
> around +/- 5ms, whereas after enabling async probing in e.g. vsp1
> driver, the variance is increased to sometimes +/- 80ms or more.
>
> This patch fixes it and keeps the startup time consistent after
> switching certain i2c-dependent drivers to asynchronous probing on
> H3-es20-Salvator-X target.
>
> Another effect seems to be improving the init time of rcar_i2c_driver
> itself from ~7ms to ~1ms (assuming CONFIG_I2C_RCAR=y).
>
> Signed-off-by: Eugeniu Rosca
> Signed-off-by: Vladimir Zapolskiy
> ---
> drivers/i2c/busses/i2c-rcar.c | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 4159ebcec2bb..502f62052b49 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -972,7 +972,18 @@ static struct platform_driver rcar_i2c_driver = {
> .remove = rcar_i2c_remove,
> };
>
> -module_platform_driver(rcar_i2c_driver);
> +static int __init rcar_i2c_driver_init(void)
> +{
> + return platform_driver_register(&rcar_i2c_driver);
> +}
> +
> +static void __exit rcar_i2c_driver_exit(void)
> +{
> + return platform_driver_unregister(&rcar_i2c_driver);
> +}
> +
> +subsys_initcall(rcar_i2c_driver_init);
> +module_exit(rcar_i2c_driver_exit);
>
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("Renesas R-Car I2C bus driver");
> --
> 2.8.1
>
Best regards,
Eugeniu.
Re: [PATCH] v4l: vsp1: Fix function declaration/definition mismatch
Hello Laurent, Kieran,
On Mon, Dec 04, 2017 at 12:52:17PM +0200, Laurent Pinchart wrote:
> Hello,
>
> On Friday, 24 November 2017 20:40:57 EET Kieran Bingham wrote:
> > Hi Eugeniu,
> >
> > Thankyou for the patch,
> >
> > Laurent - Any comments on this? Otherwise I'll bundle this in with my
> > suspend/resume patch for a pull request.
> >
> >
> >
> > I was going to say: We know the object is an entity by it's type. Isn't hgo
> > more descriptive for it's name ?
> >
> > However to answer my own question - The implementation function goes on to
> > define a struct vsp1_hgo *hgo, so no ... the Entity object shouldn't be hgo.
>
> And that's exactly why there's a difference between the declaration and
> implementation :-) Naming the parameter hgo in the declaration makes it clear
> to the reader what entity is expected. The parameter is then named entity in
> the function definition to allow for the vsp1_hgo *hgo local variable.
>
> Is the mismatch a real issue ? I don't think the kernel coding style mandates
> parameter names to be identical between function declaration and definition.
You are the DRM/VSP1 and kernel experts, so feel free to drop the patch.
Still IMO what makes kernel coding style sweet is its simplicity [1].
Here is some statistics computed with exuberant ctags and cpccheck.
$ git describe HEAD
v4.15-rc2
$ ctags --version
Exuberant Ctags 5.9~svn20110310, Copyright (C) 1996-2009 Darren Hiebert
Addresses: ,
http://ctags.sourceforge.net
Optional compiled features: +wildcards, +regex
# Number of function definitions in drivers/media:
$ find drivers/media -type d | \
xargs -I {} sh -c "ctags -x --c-types=f {}/*" | wc -l
24913
# Number of func declaration/definition mismatches found by cppcheck:
$ cppcheck --force --enable=all --inconclusive drivers/media/ 2>&1 | \
grep declaration | wc -l
168
It looks like only (168/24913) * 100% = 0,67% of all drivers/media
functions have a mismatch between function declaration and function
definition. Why making this number worse?
BR,
Eugeniu.
[1] ./Documentation/process/coding-style.rst: Kernel coding style is super
simple.
Re: [v3,15/20] drm: bridge: dw-hdmi: Handle overflow workaround based on device version
Hello Laurent, Jose,
As you might know, Renesas uses the same Synopsis Designware HDMI
transmitter IP in the Rcar Gen3 SoCs. There are v4.9+ patches present
in [1] to enable support for this.
Why I reply in this specific thread is because I am having a trial
attempt to rebase the RCAR-specific v4.9+ dw-hdmi patches on top of
a more recent kernel, which already contains mainline commit [2],
being proposed as part of this patch series.
Based on [3], it looks like that the `dw_hdmi_clear_overflow()`
workaround, formerly implemented for IMX6, is still used on Rcar3.
Current patch (integrated in upstream as commit [2]) assumes that
the workaround is only needed for the older v1.30a and v1.31a IP
implementations. Given much newer HDMI controller contained in RCAR3
(kernel reports v2.01a) and given it's you who witnessed the
original issue, I wonder what is the easiest way to check if the
problem fixed by the workaround still occurs on the Renesas SoC?
I have plenty of Salvator-X and ULCB boards, so HW is not a problem.
Your feedback is much appreciated.
Best regards,
Eugeniu.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/
[2] mainline commit be41fc55f1aa ("drm: bridge: dw-hdmi: Handle overflow
workaround based on device version")
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/commit/?h=8fdb115e6e1f
[PATCH] v4l: vsp1: Fix function declaration/definition mismatch
From: Eugeniu Rosca Cppcheck v1.81 complains that the parameter names of certain vsp1 functions don't match between declaration and definition. Fix this. No functional change is confirmed by the empty delta between the disassembled object files before and after the change. Signed-off-by: Eugeniu Rosca --- drivers/media/platform/vsp1/vsp1_entity.h | 5 +++-- drivers/media/platform/vsp1/vsp1_hgo.h| 2 +- drivers/media/platform/vsp1/vsp1_hgt.h| 2 +- drivers/media/platform/vsp1/vsp1_uds.h| 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_entity.h b/drivers/media/platform/vsp1/vsp1_entity.h index c169a060b6d2..1087d7e5c126 100644 --- a/drivers/media/platform/vsp1/vsp1_entity.h +++ b/drivers/media/platform/vsp1/vsp1_entity.h @@ -161,7 +161,8 @@ int vsp1_subdev_enum_mbus_code(struct v4l2_subdev *subdev, int vsp1_subdev_enum_frame_size(struct v4l2_subdev *subdev, struct v4l2_subdev_pad_config *cfg, struct v4l2_subdev_frame_size_enum *fse, - unsigned int min_w, unsigned int min_h, - unsigned int max_w, unsigned int max_h); + unsigned int min_width, unsigned int min_height, + unsigned int max_width, + unsigned int max_height); #endif /* __VSP1_ENTITY_H__ */ diff --git a/drivers/media/platform/vsp1/vsp1_hgo.h b/drivers/media/platform/vsp1/vsp1_hgo.h index c6c0b7a80e0c..76a9cf97b9d3 100644 --- a/drivers/media/platform/vsp1/vsp1_hgo.h +++ b/drivers/media/platform/vsp1/vsp1_hgo.h @@ -40,6 +40,6 @@ static inline struct vsp1_hgo *to_hgo(struct v4l2_subdev *subdev) } struct vsp1_hgo *vsp1_hgo_create(struct vsp1_device *vsp1); -void vsp1_hgo_frame_end(struct vsp1_entity *hgo); +void vsp1_hgo_frame_end(struct vsp1_entity *entity); #endif /* __VSP1_HGO_H__ */ diff --git a/drivers/media/platform/vsp1/vsp1_hgt.h b/drivers/media/platform/vsp1/vsp1_hgt.h index 83f2e130942a..37139d54b6c8 100644 --- a/drivers/media/platform/vsp1/vsp1_hgt.h +++ b/drivers/media/platform/vsp1/vsp1_hgt.h @@ -37,6 +37,6 @@ static inline struct vsp1_hgt *to_hgt(struct v4l2_subdev *subdev) } struct vsp1_hgt *vsp1_hgt_create(struct vsp1_device *vsp1); -void vsp1_hgt_frame_end(struct vsp1_entity *hgt); +void vsp1_hgt_frame_end(struct vsp1_entity *entity); #endif /* __VSP1_HGT_H__ */ diff --git a/drivers/media/platform/vsp1/vsp1_uds.h b/drivers/media/platform/vsp1/vsp1_uds.h index 7bf3cdcffc65..9c7f8497b5da 100644 --- a/drivers/media/platform/vsp1/vsp1_uds.h +++ b/drivers/media/platform/vsp1/vsp1_uds.h @@ -35,7 +35,7 @@ static inline struct vsp1_uds *to_uds(struct v4l2_subdev *subdev) struct vsp1_uds *vsp1_uds_create(struct vsp1_device *vsp1, unsigned int index); -void vsp1_uds_set_alpha(struct vsp1_entity *uds, struct vsp1_dl_list *dl, +void vsp1_uds_set_alpha(struct vsp1_entity *entity, struct vsp1_dl_list *dl, unsigned int alpha); #endif /* __VSP1_UDS_H__ */ -- 2.14.1
Re: [PATCH] ravb: Fix use-after-free on `ifconfig eth0 down`
On Tue, Jun 06, 2017 at 12:35:30PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 6/6/2017 1:08 AM, Eugeniu Rosca wrote:
>
> >Commit a47b70ea86bd ("ravb: unmap descriptors when freeing rings") has
> >introduced the issue seen in [1] reproduced on H3ULCB board.
> >
> >Fix this by relocating the RX skb ringbuffer free operation, so that
> >swiotlb page unmapping can be done first. Freeing of aligned TX buffers
> >is not relevant to the issue seen in [1]. Still, reposition TX free
> >calls as well, to have all kfree() operations performed consistently
> >_after_ dma_unmap_*()/dma_free_*().
>
>Perhaps it's a material of a separate cleanup patch?
Many thanks for feedback. For the moment, with a number of sanitizers
and debugging options enabled (UBSAN, KASAN, KMEMLEAK, DMA_API_DEBUG), I
couldn't find any other obvious ravb driver failures in basic usecases
(didn't stress-test it though).
Regarding the reordering of kfree vs dma_* API calls, which might be
needed in other parts of the driver, this possibly will be highlighted
by special usecases like repetitive suspend/resume or the like. I will
happily share any other fixes, if such are developed on our side.
Best regards,
Eugeniu.
[PATCH] ravb: Fix use-after-free on `ifconfig eth0 down`
Commit a47b70ea86bd ("ravb: unmap descriptors when freeing rings") has
introduced the issue seen in [1] reproduced on H3ULCB board.
Fix this by relocating the RX skb ringbuffer free operation, so that
swiotlb page unmapping can be done first. Freeing of aligned TX buffers
is not relevant to the issue seen in [1]. Still, reposition TX free
calls as well, to have all kfree() operations performed consistently
_after_ dma_unmap_*()/dma_free_*().
[1] Console screenshot with the problem reproduced:
salvator-x login: root
root@salvator-x:~# ifconfig eth0 up
Micrel KSZ9031 Gigabit PHY e680.ethernet-:00: \
attached PHY driver [Micrel KSZ9031 Gigabit PHY] \
(mii_bus:phy_addr=e680.ethernet-:00, irq=235)
IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
root@salvator-x:~#
root@salvator-x:~# ifconfig eth0 down
==
BUG: KASAN: use-after-free in swiotlb_tbl_unmap_single+0xc4/0x35c
Write of size 1538 at addr 8006d884f780 by task ifconfig/1649
CPU: 0 PID: 1649 Comm: ifconfig Not tainted 4.12.0-rc4-4-g112eb07287d1 #32
Hardware name: Renesas H3ULCB board based on r8a7795 (DT)
Call trace:
[] dump_backtrace+0x0/0x3a4
[] show_stack+0x14/0x1c
[] dump_stack+0xf8/0x150
[] print_address_description+0x7c/0x330
[] kasan_report+0x2e0/0x2f4
[] check_memory_region+0x20/0x14c
[] memcpy+0x48/0x68
[] swiotlb_tbl_unmap_single+0xc4/0x35c
[] unmap_single+0x90/0xa4
[] swiotlb_unmap_page+0xc/0x14
[] __swiotlb_unmap_page+0xcc/0xe4
[] ravb_ring_free+0x514/0x870
[] ravb_close+0x288/0x36c
[] __dev_close_many+0x14c/0x174
[] __dev_close+0xc8/0x144
[] __dev_change_flags+0xd8/0x194
[] dev_change_flags+0x60/0xb0
[] devinet_ioctl+0x484/0x9d4
[] inet_ioctl+0x190/0x194
[] sock_do_ioctl+0x78/0xa8
[] sock_ioctl+0x110/0x3c4
[] vfs_ioctl+0x90/0xa0
[] do_vfs_ioctl+0x148/0xc38
[] SyS_ioctl+0x44/0x74
[] el0_svc_naked+0x24/0x28
The buggy address belongs to the page:
page:7e001b6213c0 count:0 mapcount:0 mapping: (null) index:0x0
flags: 0x4000()
raw: 4000
raw: 7e001b6213e0
page dumped because: kasan: bad access detected
Memory state around the buggy address:
8006d884f680: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8006d884f700: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>8006d884f780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
8006d884f800: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
8006d884f880: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==
Disabling lock debugging due to kernel taint
root@salvator-x:~#
Fixes: a47b70ea86bd ("ravb: unmap descriptors when freeing rings")
Signed-off-by: Eugeniu Rosca
---
drivers/net/ethernet/renesas/ravb_main.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index 3cd7989c007d..784782da3a85 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -230,18 +230,6 @@ static void ravb_ring_free(struct net_device *ndev, int q)
int ring_size;
int i;
- /* Free RX skb ringbuffer */
- if (priv->rx_skb[q]) {
- for (i = 0; i < priv->num_rx_ring[q]; i++)
- dev_kfree_skb(priv->rx_skb[q][i]);
- }
- kfree(priv->rx_skb[q]);
- priv->rx_skb[q] = NULL;
-
- /* Free aligned TX buffers */
- kfree(priv->tx_align[q]);
- priv->tx_align[q] = NULL;
-
if (priv->rx_ring[q]) {
for (i = 0; i < priv->num_rx_ring[q]; i++) {
struct ravb_ex_rx_desc *desc = &priv->rx_ring[q][i];
@@ -270,6 +258,18 @@ static void ravb_ring_free(struct net_device *ndev, int q)
priv->tx_ring[q] = NULL;
}
+ /* Free RX skb ringbuffer */
+ if (priv->rx_skb[q]) {
+ for (i = 0; i < priv->num_rx_ring[q]; i++)
+ dev_kfree_skb(priv->rx_skb[q][i]);
+ }
+ kfree(priv->rx_skb[q]);
+ priv->rx_skb[q] = NULL;
+
+ /* Free aligned TX buffers */
+ kfree(priv->tx_align[q]);
+ priv->tx_align[q] = NULL;
+
/* Free TX skb ringbuffer.
* SKBs are freed by ravb_tx_free() call above.
*/
--
2.13.0
Re: [PATCH] pinctrl: sh-pfc: Print correct pinmux info name
On Sun, Mar 19, 2017 at 03:02:48PM +0100, Geert Uytterhoeven wrote: Hi Geert, > Thanks for your patch! > But next time, please send it inline, for easier commenting. Thanks! Will do that next time. > From a code maintenance point of view, I think it's safer to update the info > pointer itself, cfr. "[PATCH v2 1/4] pinctrl: sh-pfc: Update info pointer > after SoC-specific init" > (https://www.spinics.net/lists/linux-renesas-soc/msg12375.html). Agree! It's good that printing the correct PFC driver name is already fixed on your side. Best regards, Eugeniu.
[PATCH] pinctrl: sh-pfc: Print correct pinmux info name
>From cce7953dac965d620bb4fe5a456cffe40793f39b Mon Sep 17 00:00:00 2001
From: Eugeniu Rosca
Date: Tue, 28 Feb 2017 13:38:36 +0100
Subject: [PATCH] pinctrl: sh-pfc: Print correct pinmux info name
commit 0c151062f32c ("sh-pfc: Add support for SoC-specific
initialization") allows defining SoC specific init functions.
Such custom functions can register new pinmux info structures.
Here is an example:
static int my_pinmux_init(struct sh_pfc *pfc)
{
if (my_criteria())
pfc->info = &new_pinmux_info;
}
A side effect of the pfc->info update in the above example is that
the `const struct sh_pfc_soc_info *info` pointer used in the probe
routine becomes outdated. One consequence of it is printing the wrong
pinmux info structure name at the end of `sh_pfc_probe()`. Fix this.
Signed-off-by: Eugeniu Rosca
---
drivers/pinctrl/sh-pfc/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 6399eb1feb12..37fc70fb8e4d 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -703,7 +703,7 @@ static int sh_pfc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pfc);
- dev_info(pfc->dev, "%s support registered\n", info->name);
+ dev_info(pfc->dev, "%s support registered\n", pfc->info->name);
return 0;
}
--
2.12.0
