Re: [PATCH v5 00/16] Allwinner MIPI CSI-2 support for A31/V3s/A83T

2021-05-26 Thread Sakari Ailus
Hi Paul,

On Wed, May 26, 2021 at 03:28:20PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> On Wed 26 May 21, 14:00, Hans Verkuil wrote:
> > Hi Paul,
> > 
> > On 15/01/2021 21:01, Paul Kocialkowski wrote:
> > > This series introduces support for MIPI CSI-2, with the A31 controller 
> > > that is
> > > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific
> > > controller. While the former uses the same MIPI D-PHY that is already 
> > > supported
> > > for DSI, the latter embeds its own D-PHY.
> > > 
> > > In order to distinguish the use of the D-PHY between Rx mode (for MIPI 
> > > CSI-2)
> > > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY 
> > > API.
> > > This allows adding Rx support in the A31 D-PHY driver.
> > > 
> > > A few changes and fixes are applied to the A31 CSI controller driver, in 
> > > order
> > > to support the MIPI CSI-2 use-case.
> > 
> > Besides the compile error for patch 2/16, I also get several other compile 
> > errors:
> > 
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c: In function 
> > ‘sun6i_csi_v4l2_fwnode_init’:
> > ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> > token
> >   207 |  ((type *)   \
> >   |  ^
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:790:8: note: in 
> > expansion of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
> >   790 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
> >   |^~~~
> > ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> > token
> >   207 |  ((type *)   \
> >   |  ^
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:811:8: note: in 
> > expansion of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
> >   811 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
> >   |^~~~
> > make[5]: *** [scripts/Makefile.build:272: 
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.o] Error 1
> > make[5]: *** Waiting for unfinished jobs
> > make[4]: *** [scripts/Makefile.build:272: 
> > drivers/media/platform/rockchip/rkisp1/rkisp1-isp.o] Error 1
> > make[3]: *** [scripts/Makefile.build:515: 
> > drivers/media/platform/rockchip/rkisp1] Error 2
> > make[3]: *** Waiting for unfinished jobs
> > In file included from ./include/media/v4l2-subdev.h:14,
> >  from ./include/media/v4l2-device.h:13,
> >  from 
> > drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:19:
> > drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c: 
> > In function ‘sun8i_a83t_mipi_csi2_v4l2_setup’:
> > ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> > token
> >   207 |  ((type *)   \
> >   |  ^
> > drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:495:8:
> >  note: in expansion of macro
> > ‘v4l2_async_notifier_add_fwnode_remote_subdev’
> >   495 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, 
> > handle,
> >   |^~~~
> > In file included from ./include/media/v4l2-subdev.h:14,
> >  from ./include/media/v4l2-device.h:13,
> >  from 
> > drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:18:
> > drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c: In function 
> > ‘sun6i_mipi_csi2_v4l2_setup’:
> > ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> > token
> >   207 |  ((type *)   \
> >   |  ^
> > drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:437:8: note: 
> > in expansion of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
> >   437 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, 
> > handle,
> >   |^~~~
> > 
> > Can you rebase this series?
> 
> Thanks for letting me know, I'll look into this for the next iteration.
> 
> > I also need Acked-by's for patches 1-3 from one of the PHY maintainers, but 
> > as
> > you mentioned this might need to change as well.
> > 
> > Was there a reason why I haven't looked at this before? It's quite an old 
> > series,
> > usually I don't wait for so long. If it was because I was really slow, then 
> > I
> > apologize and please kick me sooner if you see a review like this take so 
> > long.
> 
> I'm not sure, but Sakari definitely went over previous interations and made
> various comments,so the series definitely hasn't gone unreviewed!

My acks seem to be missing. Let me go through it. As for Hans: please ping
if you don't get reviews.

-- 
Kind regards,

Sakari Ailus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 02/16] phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes

2021-05-26 Thread Paul Kocialkowski
Hi everyone,

On Fri 15 Jan 21, 21:01, Paul Kocialkowski wrote:
> As some D-PHY controllers support both Rx and Tx mode, we need a way for
> users to explicitly request one or the other. For instance, Rx mode can
> be used along with MIPI CSI-2 while Tx mode can be used with MIPI DSI.
> 
> Introduce new MIPI D-PHY PHY submodes to use with PHY_MODE_MIPI_DPHY.
> The default (zero value) is kept to Tx so only the rkisp1 driver, which
> uses D-PHY in Rx mode, needs to be adapted.

I think it was Laurent who brought up on IRC that using a submode is probably
not a correct way to distinguish between Rx and Tx modes.

Thinking about it again, it feels like selecting the direction at run-time
would only be relevant if there's D-PHY hardware than can do both Tx and Rx
*and* that can be muxed to either a MIPI DSI and a CSI-2 controller at
run-time.

For the Allwinner case, the D-PHY is the same hardware for both but there will
be one instance attached to each controller, not a single shared instance.
It feels rather unlikely that a device with both MIPI DSI and CSI-2 would only
have one PHY for the two as this wouldn't allow concurrent use of the two
controllers. Even in a case where there'd be n controllers and m < n
bi-directional PHYs, it feels safe to assume that a static attribution would
be sufficient.
 
As a result it feels more relevant to have this distinction in device-tree
rather than via the PHY API.

What do you think?
Any suggestion on how this should be represented in device-tree?

Cheers,

Paul

> Signed-off-by: Paul Kocialkowski 
> Acked-by: Helen Koike 
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c |  3 ++-
>  include/linux/phy/phy-mipi-dphy.h   | 13 +
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c 
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2e5b57e3aedc..cab261644102 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -948,7 +948,8 @@ static int rkisp1_mipi_csi2_start(struct rkisp1_isp *isp,
>  
>   phy_mipi_dphy_get_default_config(pixel_clock, isp->sink_fmt->bus_width,
>sensor->lanes, cfg);
> - phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY);
> + phy_set_mode_ext(cdev->dphy, PHY_MODE_MIPI_DPHY,
> +  PHY_MIPI_DPHY_SUBMODE_RX);
>   phy_configure(sensor->dphy, &opts);
>   phy_power_on(sensor->dphy);
>  
> diff --git a/include/linux/phy/phy-mipi-dphy.h 
> b/include/linux/phy/phy-mipi-dphy.h
> index a877ffee845d..0f57ef46a8b5 100644
> --- a/include/linux/phy/phy-mipi-dphy.h
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -6,6 +6,19 @@
>  #ifndef __PHY_MIPI_DPHY_H_
>  #define __PHY_MIPI_DPHY_H_
>  
> +/**
> + * enum phy_mipi_dphy_submode - MIPI D-PHY sub-mode
> + *
> + * A MIPI D-PHY can be used to transmit or receive data.
> + * Since some controllers can support both, the direction to enable is 
> specified
> + * with the PHY sub-mode. Transmit is assumed by default with phy_set_mode.
> + */
> +
> +enum phy_mipi_dphy_submode {
> + PHY_MIPI_DPHY_SUBMODE_TX = 0,
> + PHY_MIPI_DPHY_SUBMODE_RX,
> +};
> +
>  /**
>   * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
>   *
> -- 
> 2.30.0
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 00/16] Allwinner MIPI CSI-2 support for A31/V3s/A83T

2021-05-26 Thread Paul Kocialkowski
Hi,

On Wed 26 May 21, 14:00, Hans Verkuil wrote:
> Hi Paul,
> 
> On 15/01/2021 21:01, Paul Kocialkowski wrote:
> > This series introduces support for MIPI CSI-2, with the A31 controller that 
> > is
> > found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific
> > controller. While the former uses the same MIPI D-PHY that is already 
> > supported
> > for DSI, the latter embeds its own D-PHY.
> > 
> > In order to distinguish the use of the D-PHY between Rx mode (for MIPI 
> > CSI-2)
> > and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY 
> > API.
> > This allows adding Rx support in the A31 D-PHY driver.
> > 
> > A few changes and fixes are applied to the A31 CSI controller driver, in 
> > order
> > to support the MIPI CSI-2 use-case.
> 
> Besides the compile error for patch 2/16, I also get several other compile 
> errors:
> 
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c: In function 
> ‘sun6i_csi_v4l2_fwnode_init’:
> ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> token
>   207 |  ((type *)   \
>   |  ^
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:790:8: note: in expansion 
> of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
>   790 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
>   |^~~~
> ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> token
>   207 |  ((type *)   \
>   |  ^
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:811:8: note: in expansion 
> of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
>   811 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
>   |^~~~
> make[5]: *** [scripts/Makefile.build:272: 
> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.o] Error 1
> make[5]: *** Waiting for unfinished jobs
> make[4]: *** [scripts/Makefile.build:272: 
> drivers/media/platform/rockchip/rkisp1/rkisp1-isp.o] Error 1
> make[3]: *** [scripts/Makefile.build:515: 
> drivers/media/platform/rockchip/rkisp1] Error 2
> make[3]: *** Waiting for unfinished jobs
> In file included from ./include/media/v4l2-subdev.h:14,
>  from ./include/media/v4l2-device.h:13,
>  from 
> drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:19:
> drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c: In 
> function ‘sun8i_a83t_mipi_csi2_v4l2_setup’:
> ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> token
>   207 |  ((type *)   \
>   |  ^
> drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:495:8:
>  note: in expansion of macro
> ‘v4l2_async_notifier_add_fwnode_remote_subdev’
>   495 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, handle,
>   |^~~~
> In file included from ./include/media/v4l2-subdev.h:14,
>  from ./include/media/v4l2-device.h:13,
>  from 
> drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:18:
> drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c: In function 
> ‘sun6i_mipi_csi2_v4l2_setup’:
> ./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ 
> token
>   207 |  ((type *)   \
>   |  ^
> drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:437:8: note: 
> in expansion of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
>   437 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, handle,
>   |^~~~
> 
> Can you rebase this series?

Thanks for letting me know, I'll look into this for the next iteration.

> I also need Acked-by's for patches 1-3 from one of the PHY maintainers, but as
> you mentioned this might need to change as well.
> 
> Was there a reason why I haven't looked at this before? It's quite an old 
> series,
> usually I don't wait for so long. If it was because I was really slow, then I
> apologize and please kick me sooner if you see a review like this take so 
> long.

I'm not sure, but Sakari definitely went over previous interations and made
various comments,so the series definitely hasn't gone unreviewed!

To be honest I also lost momentum on this but I'll be trying to finalize the
series soon, once the discussion on rx/tx handling has concluded.

Cheers,

Paul

> 
> Regards,
> 
>   Hans
> 
> > 
> > Changes since v4:
> > - Added patch to stop using v4l2_async_notifier_parse_fwnode_endpoints;
> > - Fixed checkpatch strict issues (parenthesis alignment);
> > - Fixed runtime PM call order and disable;
> > - Fixed fwnode_handle_put order;
> > - Brought back phy-names for A31 since it's mandatory according to the 
> > generic
> >   PHY binding and needed by the code;
> > - Added collected tags.
> > 
> > Changes 

Re: [PATCH v5 00/16] Allwinner MIPI CSI-2 support for A31/V3s/A83T

2021-05-26 Thread Hans Verkuil
Hi Paul,

On 15/01/2021 21:01, Paul Kocialkowski wrote:
> This series introduces support for MIPI CSI-2, with the A31 controller that is
> found on most SoCs (A31, V3s and probably V5) as well as the A83T-specific
> controller. While the former uses the same MIPI D-PHY that is already 
> supported
> for DSI, the latter embeds its own D-PHY.
> 
> In order to distinguish the use of the D-PHY between Rx mode (for MIPI CSI-2)
> and Tx mode (for MIPI DSI), a submode is introduced for D-PHY in the PHY API.
> This allows adding Rx support in the A31 D-PHY driver.
> 
> A few changes and fixes are applied to the A31 CSI controller driver, in order
> to support the MIPI CSI-2 use-case.

Besides the compile error for patch 2/16, I also get several other compile 
errors:

drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c: In function 
‘sun6i_csi_v4l2_fwnode_init’:
./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ token
  207 |  ((type *)   \
  |  ^
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:790:8: note: in expansion of 
macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
  790 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
  |^~~~
./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ token
  207 |  ((type *)   \
  |  ^
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:811:8: note: in expansion of 
macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
  811 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(&csi->notifier,
  |^~~~
make[5]: *** [scripts/Makefile.build:272: 
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.o] Error 1
make[5]: *** Waiting for unfinished jobs
make[4]: *** [scripts/Makefile.build:272: 
drivers/media/platform/rockchip/rkisp1/rkisp1-isp.o] Error 1
make[3]: *** [scripts/Makefile.build:515: 
drivers/media/platform/rockchip/rkisp1] Error 2
make[3]: *** Waiting for unfinished jobs
In file included from ./include/media/v4l2-subdev.h:14,
 from ./include/media/v4l2-device.h:13,
 from 
drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:19:
drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c: In 
function ‘sun8i_a83t_mipi_csi2_v4l2_setup’:
./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ token
  207 |  ((type *)   \
  |  ^
drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c:495:8: 
note: in expansion of macro
‘v4l2_async_notifier_add_fwnode_remote_subdev’
  495 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, handle,
  |^~~~
In file included from ./include/media/v4l2-subdev.h:14,
 from ./include/media/v4l2-device.h:13,
 from 
drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:18:
drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c: In function 
‘sun6i_mipi_csi2_v4l2_setup’:
./include/media/v4l2-async.h:207:10: error: expected expression before ‘)’ token
  207 |  ((type *)   \
  |  ^
drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c:437:8: note: in 
expansion of macro ‘v4l2_async_notifier_add_fwnode_remote_subdev’
  437 |  ret = v4l2_async_notifier_add_fwnode_remote_subdev(notifier, handle,
  |^~~~

Can you rebase this series?

I also need Acked-by's for patches 1-3 from one of the PHY maintainers, but as
you mentioned this might need to change as well.

Was there a reason why I haven't looked at this before? It's quite an old 
series,
usually I don't wait for so long. If it was because I was really slow, then I
apologize and please kick me sooner if you see a review like this take so long.

Regards,

Hans

> 
> Changes since v4:
> - Added patch to stop using v4l2_async_notifier_parse_fwnode_endpoints;
> - Fixed checkpatch strict issues (parenthesis alignment);
> - Fixed runtime PM call order and disable;
> - Fixed fwnode_handle_put order;
> - Brought back phy-names for A31 since it's mandatory according to the generic
>   PHY binding and needed by the code;
> - Added collected tags.
> 
> Changes since v3:
> - Fixed single-item phys description in sun6i mipi csi-2 binding;
> - Fixed variables names in macros using container_of;
> - Fixed style issue with operators at the end of lines;
> - Reworked source endpoint/subdev assignment in sun6i-csi to handle
>   link_validate error case;
> - Removed unrelated dt change in sun8i-a83t mipi csi-2 driver;
> - Added collected tags.
> 
> Changes since v2:
> - added Kconfig depend on PM since it's not optional;
> - removed phy-names for A31 MIPI CSI-2 controller;
> - removed v3s compatible in the A31 MIPI CSI-2 controller driver;
> - removed A31 CSI controller single-port binding deprecation;
> - rem

Re: [PATCH v5 02/16] phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes

2021-05-26 Thread Paul Kocialkowski
Hi,

On Wed 26 May 21, 13:50, Hans Verkuil wrote:
> On 15/01/2021 21:01, Paul Kocialkowski wrote:
> > As some D-PHY controllers support both Rx and Tx mode, we need a way for
> > users to explicitly request one or the other. For instance, Rx mode can
> > be used along with MIPI CSI-2 while Tx mode can be used with MIPI DSI.
> > 
> > Introduce new MIPI D-PHY PHY submodes to use with PHY_MODE_MIPI_DPHY.
> > The default (zero value) is kept to Tx so only the rkisp1 driver, which
> > uses D-PHY in Rx mode, needs to be adapted.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > Acked-by: Helen Koike 
> > ---
> >  drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c |  3 ++-
> >  include/linux/phy/phy-mipi-dphy.h   | 13 +
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c 
> > b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> > index 2e5b57e3aedc..cab261644102 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> > @@ -948,7 +948,8 @@ static int rkisp1_mipi_csi2_start(struct rkisp1_isp 
> > *isp,
> >  
> > phy_mipi_dphy_get_default_config(pixel_clock, isp->sink_fmt->bus_width,
> >  sensor->lanes, cfg);
> > -   phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY);
> > +   phy_set_mode_ext(cdev->dphy, PHY_MODE_MIPI_DPHY,
> > +PHY_MIPI_DPHY_SUBMODE_RX);
> 
> drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c: In function 
> ‘rkisp1_mipi_csi2_start’:
> drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c:951:19: error: ‘cdev’ 
> undeclared (first use in this function)
>   951 |  phy_set_mode_ext(cdev->dphy, PHY_MODE_MIPI_DPHY,
>   |   ^~~~
> 
> Huh?

Oh wow that's quite shameful. Apologies here.

Note that I'll rebase/respin this series. I also remember that it wasn't very
consensual (on IRC) to use a submode to indicate rx vs tx mode and that
specifying that in the deivce-tree would be a better fit.

Cheers,

Paul

> Regards,
> 
>   Hans
> 
> > phy_configure(sensor->dphy, &opts);
> > phy_power_on(sensor->dphy);
> >  
> > diff --git a/include/linux/phy/phy-mipi-dphy.h 
> > b/include/linux/phy/phy-mipi-dphy.h
> > index a877ffee845d..0f57ef46a8b5 100644
> > --- a/include/linux/phy/phy-mipi-dphy.h
> > +++ b/include/linux/phy/phy-mipi-dphy.h
> > @@ -6,6 +6,19 @@
> >  #ifndef __PHY_MIPI_DPHY_H_
> >  #define __PHY_MIPI_DPHY_H_
> >  
> > +/**
> > + * enum phy_mipi_dphy_submode - MIPI D-PHY sub-mode
> > + *
> > + * A MIPI D-PHY can be used to transmit or receive data.
> > + * Since some controllers can support both, the direction to enable is 
> > specified
> > + * with the PHY sub-mode. Transmit is assumed by default with phy_set_mode.
> > + */
> > +
> > +enum phy_mipi_dphy_submode {
> > +   PHY_MIPI_DPHY_SUBMODE_TX = 0,
> > +   PHY_MIPI_DPHY_SUBMODE_RX,
> > +};
> > +
> >  /**
> >   * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
> >   *
> > 
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v5 02/16] phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes

2021-05-26 Thread Hans Verkuil
On 15/01/2021 21:01, Paul Kocialkowski wrote:
> As some D-PHY controllers support both Rx and Tx mode, we need a way for
> users to explicitly request one or the other. For instance, Rx mode can
> be used along with MIPI CSI-2 while Tx mode can be used with MIPI DSI.
> 
> Introduce new MIPI D-PHY PHY submodes to use with PHY_MODE_MIPI_DPHY.
> The default (zero value) is kept to Tx so only the rkisp1 driver, which
> uses D-PHY in Rx mode, needs to be adapted.
> 
> Signed-off-by: Paul Kocialkowski 
> Acked-by: Helen Koike 
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c |  3 ++-
>  include/linux/phy/phy-mipi-dphy.h   | 13 +
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c 
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2e5b57e3aedc..cab261644102 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -948,7 +948,8 @@ static int rkisp1_mipi_csi2_start(struct rkisp1_isp *isp,
>  
>   phy_mipi_dphy_get_default_config(pixel_clock, isp->sink_fmt->bus_width,
>sensor->lanes, cfg);
> - phy_set_mode(sensor->dphy, PHY_MODE_MIPI_DPHY);
> + phy_set_mode_ext(cdev->dphy, PHY_MODE_MIPI_DPHY,
> +  PHY_MIPI_DPHY_SUBMODE_RX);

drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c: In function 
‘rkisp1_mipi_csi2_start’:
drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c:951:19: error: ‘cdev’ 
undeclared (first use in this function)
  951 |  phy_set_mode_ext(cdev->dphy, PHY_MODE_MIPI_DPHY,
  |   ^~~~

Huh?

Regards,

Hans

>   phy_configure(sensor->dphy, &opts);
>   phy_power_on(sensor->dphy);
>  
> diff --git a/include/linux/phy/phy-mipi-dphy.h 
> b/include/linux/phy/phy-mipi-dphy.h
> index a877ffee845d..0f57ef46a8b5 100644
> --- a/include/linux/phy/phy-mipi-dphy.h
> +++ b/include/linux/phy/phy-mipi-dphy.h
> @@ -6,6 +6,19 @@
>  #ifndef __PHY_MIPI_DPHY_H_
>  #define __PHY_MIPI_DPHY_H_
>  
> +/**
> + * enum phy_mipi_dphy_submode - MIPI D-PHY sub-mode
> + *
> + * A MIPI D-PHY can be used to transmit or receive data.
> + * Since some controllers can support both, the direction to enable is 
> specified
> + * with the PHY sub-mode. Transmit is assumed by default with phy_set_mode.
> + */
> +
> +enum phy_mipi_dphy_submode {
> + PHY_MIPI_DPHY_SUBMODE_TX = 0,
> + PHY_MIPI_DPHY_SUBMODE_RX,
> +};
> +
>  /**
>   * struct phy_configure_opts_mipi_dphy - MIPI D-PHY configuration set
>   *
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel