Re: [PATCH v9 1/4] phy: cadence: Add Cadence D-PHY Rx driver

[Pratyush Yadav]

On 22/02/22 07:40PM, Vinod Koul wrote:
> On 31-01-22, 23:03, Pratyush Yadav wrote:
> > The Cadence D-PHY can be configured in Tx (DSI) mode or Rx (CSI) mode.
> > Both modes have a different programming sequence and share little among
> > them. In addition, a PHY configured in Tx mode cannot be used in Rx mode
> > and vice versa. For this reason, create a separate driver for the Rx
> > mode to make it easier to read and maintain.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@xxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > 
> > ---
> > 
> > Changes in v9:
> > - Use %ld instead of %d for printing PTR_ERR().
> > 
> > Changes in v8:
> > - Move lanes check to start of configure sequence.
> > - Change MODULE_LICENSE() to "GPL".
> > - Add Laurent's R-by.
> > 
> > Changes in v7:
> > - Add spaces after { and before } in the bands table.
> > - Drop the wrapping around the for loop on cdns_dphy_rx_get_band_ctrl().
> > - Make cdns_dphy_rx_wait_for_bit() inline.
> > - Print an error message if registering PHY provider fails.
> > 
> > Changes in v6:
> > - Move to a separate driver.
> > 
> > Changes in v5:
> > - Use the new cdns_dphy_info to specify PHY ops.
> > - Re-order include in alphabetical order.
> > - Make bands const.
> > - Drop num_bands.
> > - Make i, lanes unsigned.
> > - Drop the maximum check in cdns_dphy_rx_get_band_ctrl(). Let the loop
> >   complete and return -EOPNOTSUPP when we reach the end.
> > - Drop the "rate < bands[i].min_rate" check since the bands are in
> >   ascending order.
> > - Move data_lane_ctrl to start of function and make it static const.
> > 
> > Changes in v4:
> > - Drop the submode parts. Use a different compatible for the Rx ops.
> > - Make bands and num_bands static.
> > 
> > Changes in v3:
> > - Use a table to select the band.
> > - Use a table to poll the data lane ready bits.
> > - Multiply the DPHY HS clock rate by 2 to get the bit rate since the
> >   clock is DDR.
> > 
> >  drivers/phy/cadence/Kconfig        |   8 +
> >  drivers/phy/cadence/Makefile       |   1 +
> >  drivers/phy/cadence/cdns-dphy-rx.c | 255 +++++++++++++++++++++++++++++
> >  3 files changed, 264 insertions(+)
> >  create mode 100644 drivers/phy/cadence/cdns-dphy-rx.c
> > 
> > diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig
> > index a62910ff5591..1adde2d99ae7 100644
> > --- a/drivers/phy/cadence/Kconfig
> > +++ b/drivers/phy/cadence/Kconfig
> > @@ -22,6 +22,14 @@ config PHY_CADENCE_DPHY
> >  	  system. If M is selected, the module will be called
> >  	  cdns-dphy.
> >  
> > +config PHY_CADENCE_DPHY_RX
> > +	tristate "Cadence D-PHY Rx Support"
> > +	depends on HAS_IOMEM && OF
> > +	select GENERIC_PHY
> > +	select GENERIC_PHY_MIPI_DPHY
> > +	help
> > +	  Support for Cadence D-PHY in Rx configuration.
> > +
> >  config PHY_CADENCE_SIERRA
> >  	tristate "Cadence Sierra PHY Driver"
> >  	depends on OF && HAS_IOMEM && RESET_CONTROLLER
> > diff --git a/drivers/phy/cadence/Makefile b/drivers/phy/cadence/Makefile
> > index 26e16bd34efe..e17f035ddece 100644
> > --- a/drivers/phy/cadence/Makefile
> > +++ b/drivers/phy/cadence/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-$(CONFIG_PHY_CADENCE_TORRENT)	+= phy-cadence-torrent.o
> >  obj-$(CONFIG_PHY_CADENCE_DPHY)	+= cdns-dphy.o
> > +obj-$(CONFIG_PHY_CADENCE_DPHY_RX)	+= cdns-dphy-rx.o
> >  obj-$(CONFIG_PHY_CADENCE_SIERRA)	+= phy-cadence-sierra.o
> >  obj-$(CONFIG_PHY_CADENCE_SALVO)	+= phy-cadence-salvo.o
> > diff --git a/drivers/phy/cadence/cdns-dphy-rx.c b/drivers/phy/cadence/cdns-dphy-rx.c
> > new file mode 100644
> > index 000000000000..c9bb8c7f16f6
> > --- /dev/null
> > +++ b/drivers/phy/cadence/cdns-dphy-rx.c
> > @@ -0,0 +1,255 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> 
> GPL v2 and more...
> 
> > +/*
> > + * Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com/
> 
> this should say 2022 as well
> 
> > +MODULE_AUTHOR("Pratyush Yadav <p.yadav@xxxxxx>");
> > +MODULE_DESCRIPTION("Cadence D-PHY Rx Driver");
> > +MODULE_LICENSE("GPL");
> 
> This means GPL v2 only and does not match SPDX tag, pls update

Will change SPDX to GPL-2.0-only.

> 
> -- 
> ~Vinod

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.