Re: [PATCH v12 01/11] media: staging: phy-rockchip-dphy: add Rockchip MIPI Synopsys DPHY driver

[Ezequiel Garcia]

Hi Helen,

Thanks for taking care of this.

On Fri, 2019-12-27 at 17:01 -0300, Helen Koike wrote:
> From: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> 
> Add driver for Rockchip MIPI Synopsys DPHY driver
> 
> Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> Signed-off-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
> 
> ---
> 
> Changes in v12:
> - several cleanups
> - remove "rx" from function names, as this driver only supports rx
> 
> Changes in v11:
> - fix checkpatch errors
> 
> Changes in v10: None
> Changes in v9:
> - Move to staging
> - replace memcpy by a directly assignment
> - remove unecessary ret variable in rockchip_dphy_init
> - s/0x1/1
> - s/0x0/0
> - coding style changes
> - dphy_reg variable sizes
> - variables from int to unsigned int
> - rename functions to start with rk_
> - rename dphy0 to rx
> - fix hardcoded lane0 usage
> - disable rx on power off
> - general cleanups of unused variables
> 
> Changes in v8:
> - Remove boiler plate license text
> 
> Changes in v7:
> - Migrate dphy specific code from
> drivers/media/platform/rockchip/isp1/mipi_dphy_sy.c
> to drivers/phy/rockchip/phy-rockchip-dphy.c
> - Drop support for rk3288
> - Drop support for dphy txrx
> - code styling and checkpatch fixes
> 
>  drivers/staging/media/Kconfig                 |   2 +
>  drivers/staging/media/Makefile                |   1 +
>  .../staging/media/phy-rockchip-dphy/Kconfig   |  11 +
>  .../staging/media/phy-rockchip-dphy/Makefile  |   2 +
>  drivers/staging/media/phy-rockchip-dphy/TODO  |   6 +
>  .../phy-rockchip-dphy/phy-rockchip-dphy.c     | 396 ++++++++++++++++++
>  6 files changed, 418 insertions(+)
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/Kconfig
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/Makefile
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/TODO
>  create mode 100644 drivers/staging/media/phy-rockchip-dphy/phy-rockchip-dphy.c
> 
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index 642adc4c24d2..a47484473883 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -38,4 +38,6 @@ source "drivers/staging/media/ipu3/Kconfig"
>  
>  source "drivers/staging/media/soc_camera/Kconfig"
>  
> +source "drivers/staging/media/phy-rockchip-dphy/Kconfig"
> +
>  endif
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index 2f1711a8aeed..b0eae3906208 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_TEGRA_VDE)		+= tegra-vde/
>  obj-$(CONFIG_VIDEO_HANTRO)	+= hantro/
>  obj-$(CONFIG_VIDEO_IPU3_IMGU)	+= ipu3/
>  obj-$(CONFIG_SOC_CAMERA)	+= soc_camera/
> +obj-$(CONFIG_PHY_ROCKCHIP_DPHY)		+= phy-rockchip-dphy/
> diff --git a/drivers/staging/media/phy-rockchip-dphy/Kconfig b/drivers/staging/media/phy-rockchip-dphy/Kconfig
> new file mode 100644
> index 000000000000..7378bd75fa7c
> --- /dev/null

Seems I overlooked the Kconfig file for this driver,
sorry about that!

> +++ b/drivers/staging/media/phy-rockchip-dphy/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Phy drivers for Rockchip platforms
> +#
> +config PHY_ROCKCHIP_DPHY
> +	tristate "Rockchip MIPI Synopsys DPHY driver"
> +	depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF
> +	select GENERIC_PHY_MIPI_DPHY
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the Rockchip MIPI Synopsys DPHY.

Following a more user-friendly convention, this should read
more like:

"""
Enable this to support the Rockchip MIPI Synopsys DPHY
associated to the Rockchip ISP module present in RK3399 SoCs.

To compile this driver as a module, choose M here: the module
will be called phy-rockchip-dphy. 
"""

And I believe the same improvement should be applied to the
ISP driver Kconfig help.

Thanks,
Ezequiel