Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines

[Nishanth Menon]

On 15:00-20200917, Roger Quadros wrote:
> Hi Peter & Nishanth,
> 
> On 16/09/2020 18:45, Nishanth Menon wrote:
> > On 06:52-20200916, Peter Rosin wrote:
> > > Hi,
> > > 
> > > Sorry for the delay.
> > > 
> > > On 2020-09-15 13:20, Roger Quadros wrote:
> > > > Each SERDES lane mux can select upto 4 different IPs.
> > > > There are 4 lanes in each J7200 SERDES. Define all
> > > > the possible functions in this file.
> > > > 
> > > > Cc: Peter Rosin <peda@xxxxxxxxxx>
> > > > Signed-off-by: Roger Quadros <rogerq@xxxxxx>
> > > > ---
> > > >   include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
> > > >   1 file changed, 29 insertions(+)
> > > >   create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
> > > > 
> > > > diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
> > > > new file mode 100644
> > > > index 000000000000..b091b1185a36
> > > > --- /dev/null
> > > > +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
> > > > @@ -0,0 +1,29 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * This header provides constants for J7200 WIZ.
> > > > + */
> > > > +
> > > > +#ifndef _DT_BINDINGS_J7200_WIZ
> > > > +#define _DT_BINDINGS_J7200_WIZ
> > > > +
> > > > +#define SERDES0_LANE0_QSGMII_LANE3	0x0
> > > > +#define SERDES0_LANE0_PCIE1_LANE0	0x1
> > > > +#define SERDES0_LANE0_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE0_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE1_QSGMII_LANE4	0x0
> > > > +#define SERDES0_LANE1_PCIE1_LANE1	0x1
> > > > +#define SERDES0_LANE1_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE1_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE2_QSGMII_LANE1	0x0
> > > > +#define SERDES0_LANE2_PCIE1_LANE2	0x1
> > > > +#define SERDES0_LANE2_IP3_UNUSED	0x2
> > > > +#define SERDES0_LANE2_IP4_UNUSED	0x3
> > > > +
> > > > +#define SERDES0_LANE3_QSGMII_LANE2	0x0
> > > > +#define SERDES0_LANE3_PCIE1_LANE3	0x1
> > > > +#define SERDES0_LANE3_USB		0x2
> > > > +#define SERDES0_LANE3_IP4_UNUSED	0x3
> > > > +
> > > > +#endif /* _DT_BINDINGS_J7200_WIZ */
> > > 
> > > Should not the defines start with J7200_WIZ? SERDES0 seems like a too
> > > generic prefix, at least to me.
> > 
> > Thanks, good point. I am not sure if WIZ should even be used.. It is
> > a TI internal prefix for various serdes solutions, but I agree that
> > SERDES0 is too generic a terminology. That said, we should cleanup
> > include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
> > j7200 changes.
> > 
> 
> I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
> and add SOC specific prefixes to the macros.
> 
> This will mean some churn in the existing DT files. (only 2 so far)

Please check bindings and examples if any reference as well. Those
changes will need to be considered as well.

> 
> Are you guys OK if I do the change in one patch to avoid a broken build in between.
> You guys can then decide whose tree it goes through.
> 
> The new SoC addition will be separate of course.

If Peter acks and is OK with the changes, then based on Peter's opinion,
I'd rather take the changes via SoC tree for 5.10+ for maintaining
bisectability.

I prefer we name it ti-serdes-mux or something that Peter is OK with as
well. reasons:

i) "wiz" is yet another TLA deal even if documented in public TI TRM in some
   remote chapter, other non-TI folks are going to go scratching their
   heads..
ii) There is no way this can scale with one header per SoC!

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D