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

[Lukasz Majewski <lukma@denx.de>]

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

Hi Tom,

> 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?
> 

Yes, it should.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]