Re: [PATCH 2/2] dts: Add u-boot specific 'fsl,mux_mask' property to iomuxc for vf610

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]

On Wed, Jun 25, 2025 at 10:32:50PM +0200, Lukasz Majewski wrote:
> Hi Tom,
> 
> > On Wed, Jun 25, 2025 at 05:28:56PM +0100, Conor Dooley wrote:
> > > On Wed, Jun 25, 2025 at 08:14:24AM -0600, Tom Rini wrote:  
> > > > On Wed, Jun 25, 2025 at 08:36:37AM +0200, Lukasz Majewski wrote:  
> > > > > Hi Tom,
> > > > >   
> > > > > > On Tue, Jun 24, 2025 at 10:47:00PM +0200, Lukasz Majewski
> > > > > > wrote: 
> > > > > > > The commit e8a9521e649f
> > > > > > > ("vf500/vf610: synchronise device trees with linux")
> > > > > > > has synchronized U-Boot's DTS with v5.19 Linux kernel.
> > > > > > > It turned out that in Linux's upstream iomuxc node
> > > > > > > description the fsl,mux_mask' was missing, so the U-Boot's
> > > > > > > pinctrl driver for NXP's Vybrid SoC was not working
> > > > > > > properly.
> > > > > > > 
> > > > > > > As by default the mux mask was set to 0, the vf610 based
> > > > > > > boards (like BK4) were bricked, due to misconfiguration of
> > > > > > > gpio at early boot stage.
> > > > > > > 
> > > > > > > The fix for all vf610 based boards is to introduce
> > > > > > > vfxxx-u-boot.dtsi file with 'fsl,mux_mask' property
> > > > > > > provided and include it in boards' specific U-Boot
> > > > > > > adjustment files (like vf610-bk4r1-u-boot.dtsi).
> > > > > > > 
> > > > > > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > > > > > ---
> > > > > > >  arch/arm/dts/vf610-bk4r1-u-boot.dtsi | 2 ++
> > > > > > >  arch/arm/dts/vfxxx-u-boot.dtsi       | 9 +++++++++
> > > > > > >  2 files changed, 11 insertions(+)
> > > > > > >  create mode 100644 arch/arm/dts/vfxxx-u-boot.dtsi    
> > > > > > 
> > > > > > It looks like this is still missing upstream, so what's the
> > > > > > status on that?   
> > > > > 
> > > > > It looks like in Linux the mux_mask is hardcoded (for Vybrid
> > > > > vf610):
> > > > > https://elixir.bootlin.com/linux/v6.16-rc3/source/drivers/pinctrl/freescale/pinctrl-vf610.c#L321
> > > > > 
> > > > > In u-boot other SoCs use it as well, but with different values:
> > > > > - arch/arm/dts/imxrt1050.dtsi -> 0x7
> > > > > - arch/arm/dts/imx8ulp-evk-u-boot.dtsi -> 0xf00
> > > > > 
> > > > > In the imx8ulp case above - it is already set in *-u-boot.dtsi
> > > > > specific file, so I've followed this approach.  
> > > > 
> > > > Someone should unify upstream to one approach or another I would
> > > > think. Or find a good explanation as to why it's not unified, and
> > > > then we should follow.  
> > > 
> > > If these values are per-SoC, which they appear to be given these
> > > look like 3 different devices, I don't see a reason for these to be
> > > accepted "upstream".  
> > 
> > OK, yes. But it should either be "hard code a thing" or "get the
> > property" not maybe-one-maybe-the-other? That was what I was trying to
> > make as my point.
> > 
> 
> Agreed.
> 
> Then some decision shall be made if:
> 
> 1. We keep the fsl,mux-mask as *-u-boot.dtsi specific
> 
> 2. Modify the pinctrl driver for Vybrid
> (drivers/pinctrl/nxp/pinctrl-imx-mmio.c) and maybe add mux_mask field
> to:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/nxp/pinctrl-imx.h?ref_type=heads#L26
> 
> 3. Try to upstream (to Linux) the property. That would require rewriting
> kernel pinctrl driver for vf610 to parse "fsl,mux_mask" property.
> 
> As Conor pointed out - the approach from 3. is not feasible.
> 
> Then we shall decide if 1. or 2. shall be implemented.
> 
> Approach from 1. doesn't require any driver's modification. Approach
> from 2. would introduce the extra field and some code modification to:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/nxp/pinctrl-imx.c?ref_type=heads
> and
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pinctrl/nxp/pinctrl-imx-mmio.c?ref_type=heads
> 
> I would personally opt for 1.

Well, my question is wouldn't option 2 bring us closer to how this works
in Linux?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]