Re: [PATCH] arm64: dts: ti: k3-j721e: Rename mux header and update macro names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi!

A few questions. I don't really care, but if someone does then at least I
highlighted few thing out.

On 2020-09-17 18:16, Roger Quadros wrote:
> We intend to use one header file for SERDES MUX for all
> TI SoCs so rename the header file.
> 
> The exsting macros are too generic. Prefix them with SoC name.
> 
> Signed-off-by: Roger Quadros <rogerq@xxxxxx>
> ---
>  .../dts/ti/k3-j721e-common-proc-board.dts     | 11 ++--
>  arch/arm64/boot/dts/ti/k3-j721e-main.dtsi     | 13 +++--
>  include/dt-bindings/mux/mux-j721e-wiz.h       | 53 ------------------
>  include/dt-bindings/mux/ti-serdes.h           | 55 +++++++++++++++++++
>  4 files changed, 68 insertions(+), 64 deletions(-)
>  delete mode 100644 include/dt-bindings/mux/mux-j721e-wiz.h
>  create mode 100644 include/dt-bindings/mux/ti-serdes.h
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
> index 8bc1e6ecc50e..493f64ee7a2a 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
> @@ -404,11 +404,12 @@
>  };
>  
>  &serdes_ln_ctrl {
> -	idle-states = <SERDES0_LANE0_PCIE0_LANE0>, <SERDES0_LANE1_PCIE0_LANE1>,
> -		      <SERDES1_LANE0_PCIE1_LANE0>, <SERDES1_LANE1_PCIE1_LANE1>,
> -		      <SERDES2_LANE0_PCIE2_LANE0>, <SERDES2_LANE1_PCIE2_LANE1>,
> -		      <SERDES3_LANE0_USB3_0_SWAP>, <SERDES3_LANE1_USB3_0>,
> -		      <SERDES4_LANE0_EDP_LANE0>, <SERDES4_LANE1_EDP_LANE1>, <SERDES4_LANE2_EDP_LANE2>, <SERDES4_LANE3_EDP_LANE3>;
> +	idle-states = <J721E_SERDES0_LANE0_PCIE0_LANE0>, <J721E_SERDES0_LANE1_PCIE0_LANE1>,
> +		      <J721E_SERDES1_LANE0_PCIE1_LANE0>, <J721E_SERDES1_LANE1_PCIE1_LANE1>,
> +		      <J721E_SERDES2_LANE0_PCIE2_LANE0>, <J721E_SERDES2_LANE1_PCIE2_LANE1>,
> +		      <J721E_SERDES3_LANE0_USB3_0_SWAP>, <J721E_SERDES3_LANE1_USB3_0>,
> +		      <J721E_SERDES4_LANE0_EDP_LANE0>, <J721E_SERDES4_LANE1_EDP_LANE1>,
> +		      <J721E_SERDES4_LANE2_EDP_LANE2>, <J721E_SERDES4_LANE3_EDP_LANE3>;
>  };
>  
>  &serdes_wiz3 {
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> index d14060207f00..924c51146d01 100644
> --- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
> @@ -6,7 +6,7 @@
>   */
>  #include <dt-bindings/phy/phy.h>
>  #include <dt-bindings/mux/mux.h>
> -#include <dt-bindings/mux/mux-j721e-wiz.h>
> +#include <dt-bindings/mux/ti-serdes.h>
>  
>  &cbass_main {
>  	msmc_ram: sram@70000000 {
> @@ -38,11 +38,12 @@
>  					<0x40b0 0x3>, <0x40b4 0x3>, /* SERDES3 lane0/1 select */
>  					<0x40c0 0x3>, <0x40c4 0x3>, <0x40c8 0x3>, <0x40cc 0x3>;
>  					/* SERDES4 lane0/1/2/3 select */
> -			idle-states = <SERDES0_LANE0_PCIE0_LANE0>, <SERDES0_LANE1_PCIE0_LANE1>,
> -				      <SERDES1_LANE0_PCIE1_LANE0>, <SERDES1_LANE1_PCIE1_LANE1>,
> -				      <SERDES2_LANE0_PCIE2_LANE0>, <SERDES2_LANE1_PCIE2_LANE1>,
> -				      <MUX_IDLE_AS_IS>, <SERDES3_LANE1_USB3_0>,
> -				      <SERDES4_LANE0_EDP_LANE0>, <SERDES4_LANE1_EDP_LANE1>, <SERDES4_LANE2_EDP_LANE2>, <SERDES4_LANE3_EDP_LANE3>;
> +			idle-states = <J721E_SERDES0_LANE0_PCIE0_LANE0>, <J721E_SERDES0_LANE1_PCIE0_LANE1>,
> +				      <J721E_SERDES1_LANE0_PCIE1_LANE0>, <J721E_SERDES1_LANE1_PCIE1_LANE1>,
> +				      <J721E_SERDES2_LANE0_PCIE2_LANE0>, <J721E_SERDES2_LANE1_PCIE2_LANE1>,
> +				      <MUX_IDLE_AS_IS>, <J721E_SERDES3_LANE1_USB3_0>,
> +				      <J721E_SERDES4_LANE0_EDP_LANE0>, <J721E_SERDES4_LANE1_EDP_LANE1>,
> +				      <J721E_SERDES4_LANE2_EDP_LANE2>, <J721E_SERDES4_LANE3_EDP_LANE3>;
>  		};
>  
>  		usb_serdes_mux: mux-controller@4000 {
> diff --git a/include/dt-bindings/mux/mux-j721e-wiz.h b/include/dt-bindings/mux/mux-j721e-wiz.h
> deleted file mode 100644
> index fd1c4ea9fc7f..000000000000
> --- a/include/dt-bindings/mux/mux-j721e-wiz.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * This header provides constants for J721E WIZ.
> - */
> -
> -#ifndef _DT_BINDINGS_J721E_WIZ
> -#define _DT_BINDINGS_J721E_WIZ
> -
> -#define SERDES0_LANE0_QSGMII_LANE1	0x0
> -#define SERDES0_LANE0_PCIE0_LANE0	0x1
> -#define SERDES0_LANE0_USB3_0_SWAP	0x2
> -
> -#define SERDES0_LANE1_QSGMII_LANE2	0x0
> -#define SERDES0_LANE1_PCIE0_LANE1	0x1
> -#define SERDES0_LANE1_USB3_0		0x2
> -
> -#define SERDES1_LANE0_QSGMII_LANE3	0x0
> -#define SERDES1_LANE0_PCIE1_LANE0	0x1
> -#define SERDES1_LANE0_USB3_1_SWAP	0x2
> -#define SERDES1_LANE0_SGMII_LANE0	0x3
> -
> -#define SERDES1_LANE1_QSGMII_LANE4	0x0
> -#define SERDES1_LANE1_PCIE1_LANE1	0x1
> -#define SERDES1_LANE1_USB3_1		0x2
> -#define SERDES1_LANE1_SGMII_LANE1	0x3
> -
> -#define SERDES2_LANE0_PCIE2_LANE0	0x1
> -#define SERDES2_LANE0_SGMII_LANE0	0x3
> -#define SERDES2_LANE0_USB3_1_SWAP	0x2
> -
> -#define SERDES2_LANE1_PCIE2_LANE1	0x1
> -#define SERDES2_LANE1_USB3_1		0x2
> -#define SERDES2_LANE1_SGMII_LANE1	0x3
> -
> -#define SERDES3_LANE0_PCIE3_LANE0	0x1
> -#define SERDES3_LANE0_USB3_0_SWAP	0x2
> -
> -#define SERDES3_LANE1_PCIE3_LANE1	0x1
> -#define SERDES3_LANE1_USB3_0		0x2
> -
> -#define SERDES4_LANE0_EDP_LANE0		0x0
> -#define SERDES4_LANE0_QSGMII_LANE5	0x2
> -
> -#define SERDES4_LANE1_EDP_LANE1		0x0
> -#define SERDES4_LANE1_QSGMII_LANE6	0x2
> -
> -#define SERDES4_LANE2_EDP_LANE2		0x0
> -#define SERDES4_LANE2_QSGMII_LANE7	0x2
> -
> -#define SERDES4_LANE3_EDP_LANE3		0x0
> -#define SERDES4_LANE3_QSGMII_LANE8	0x2
> -
> -#endif /* _DT_BINDINGS_J721E_WIZ */
> diff --git a/include/dt-bindings/mux/ti-serdes.h b/include/dt-bindings/mux/ti-serdes.h
> new file mode 100644
> index 000000000000..3e1f2d243e4a
> --- /dev/null
> +++ b/include/dt-bindings/mux/ti-serdes.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides constants for SERDES MUX for TI SoCs
> + */
> +
> +#ifndef _DT_BINDINGS_TI_SERDES_MUX
> +#define _DT_BINDINGS_TI_SERDES_MUX

I would have spelled this _DT_BINDINGS_MUX_TI_SERDES. But as stated, it
doesn't really matter to me.

> +
> +/* J721E */
> +
> +#define J721E_SERDES0_LANE0_QSGMII_LANE1	0x0
> +#define J721E_SERDES0_LANE0_PCIE0_LANE0		0x1
> +#define J721E_SERDES0_LANE0_USB3_0_SWAP		0x2
> +
> +#define J721E_SERDES0_LANE1_QSGMII_LANE2	0x0
> +#define J721E_SERDES0_LANE1_PCIE0_LANE1		0x1
> +#define J721E_SERDES0_LANE1_USB3_0		0x2
> +
> +#define J721E_SERDES1_LANE0_QSGMII_LANE3	0x0
> +#define J721E_SERDES1_LANE0_PCIE1_LANE0		0x1
> +#define J721E_SERDES1_LANE0_USB3_1_SWAP		0x2
> +#define J721E_SERDES1_LANE0_SGMII_LANE0		0x3
> +
> +#define J721E_SERDES1_LANE1_QSGMII_LANE4	0x0
> +#define J721E_SERDES1_LANE1_PCIE1_LANE1		0x1
> +#define J721E_SERDES1_LANE1_USB3_1		0x2
> +#define J721E_SERDES1_LANE1_SGMII_LANE1		0x3
> +
> +#define J721E_SERDES2_LANE0_PCIE2_LANE0		0x1
> +#define J721E_SERDES2_LANE0_SGMII_LANE0		0x3
> +#define J721E_SERDES2_LANE0_USB3_1_SWAP		0x2
> +
> +#define J721E_SERDES2_LANE1_PCIE2_LANE1		0x1
> +#define J721E_SERDES2_LANE1_USB3_1		0x2
> +#define J721E_SERDES2_LANE1_SGMII_LANE1		0x3
> +
> +#define J721E_SERDES3_LANE0_PCIE3_LANE0		0x1
> +#define J721E_SERDES3_LANE0_USB3_0_SWAP		0x2
> +
> +#define J721E_SERDES3_LANE1_PCIE3_LANE1		0x1
> +#define J721E_SERDES3_LANE1_USB3_0		0x2
> +
> +#define J721E_SERDES4_LANE0_EDP_LANE0		0x0
> +#define J721E_SERDES4_LANE0_QSGMII_LANE5	0x2
> +
> +#define J721E_SERDES4_LANE1_EDP_LANE1		0x0
> +#define J721E_SERDES4_LANE1_QSGMII_LANE6	0x2
> +
> +#define J721E_SERDES4_LANE2_EDP_LANE2		0x0
> +#define J721E_SERDES4_LANE2_QSGMII_LANE7	0x2
> +
> +#define J721E_SERDES4_LANE3_EDP_LANE3		0x0
> +#define J721E_SERDES4_LANE3_QSGMII_LANE8	0x2
> +
> +#endif /* _DT_BINDINGS_TI_SERDES_MUX */
> 

The J7200-series listed *all* possible mux values, some with names
like BLA_BLA_UNUSED. Why is that different here? Some dt-files using
the J7200 then ended up using these "unused" entries (for idle-states)
so maybe thoes values are useful here as well? The choice of using
_UNUSED for entries that end up being used can of course also be
debated :-)

If it is ill-adviced to use the values not listed above, that's
another matter of course...

I don't know the answer to the above, and will not be impacted in the
least, I'm just throwing out questions. So, either way:

Acked-by: Peter Rosin <peda@xxxxxxxxxx>

Cheers,
Peter



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux