Re: [PATCH v8 06/18] media: qcom: camss: Add support for PHY API devices

[Christopher Obbard <christopher.obbard@linaro.org>]

Hi Bryan,

On Wed, 2026-02-25 at 15:11 +0000, Bryan O'Donoghue wrote:
> Add the ability to use a PHY pointer which interacts with the standard PHY
> API.
> 
> In the first instance the code will try to use the new PHY interface. If no
> PHYs are present in the DT then the legacy method will be attempted.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Christopher Obbard <christopher.obbard@linaro.org>
Tested-by: Christopher Obbard <christopher.obbard@linaro.org>

> ---
>  drivers/media/platform/qcom/camss/Kconfig        |   1 +
>  drivers/media/platform/qcom/camss/camss-csiphy.c | 185 +++++++++++++++++++++--
>  drivers/media/platform/qcom/camss/camss-csiphy.h |   7 +
>  drivers/media/platform/qcom/camss/camss.c        |  72 +++++++--
>  4 files changed, 235 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/Kconfig b/drivers/media/platform/qcom/camss/Kconfig
> index 4eda48cb1adf0..1edc5e5a1829e 100644
> --- a/drivers/media/platform/qcom/camss/Kconfig
> +++ b/drivers/media/platform/qcom/camss/Kconfig
> @@ -7,3 +7,4 @@ config VIDEO_QCOM_CAMSS
>  	select VIDEO_V4L2_SUBDEV_API
>  	select VIDEOBUF2_DMA_SG
>  	select V4L2_FWNODE
> +	select PHY_QCOM_MIPI_CSI2
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 62623393f4144..59564284fd270 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -7,12 +7,14 @@
>   * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
>   * Copyright (C) 2016-2018 Linaro Ltd.
>   */
> +#include <dt-bindings/phy/phy.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/of.h>
> +#include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <media/media-entity.h>
> @@ -131,10 +133,10 @@ static u8 csiphy_get_bpp(const struct csiphy_format_info *formats,
>  }
>  
>  /*
> - * csiphy_set_clock_rates - Calculate and set clock rates on CSIPHY module
> + * csiphy_set_clock_rates_legacy - Calculate and set clock rates on CSIPHY module
>   * @csiphy: CSIPHY device
>   */
> -static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
> +static int csiphy_set_clock_rates_legacy(struct csiphy_device *csiphy)
>  {
>  	struct device *dev = csiphy->camss->dev;
>  	s64 link_freq;
> @@ -200,7 +202,7 @@ static int csiphy_set_clock_rates(struct csiphy_device *csiphy)
>   *
>   * Return 0 on success or a negative error code otherwise
>   */
> -static int csiphy_set_power(struct v4l2_subdev *sd, int on)
> +static int csiphy_set_power_legacy(struct v4l2_subdev *sd, int on)
>  {
>  	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
>  	struct device *dev = csiphy->camss->dev;
> @@ -219,7 +221,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
>  			return ret;
>  		}
>  
> -		ret = csiphy_set_clock_rates(csiphy);
> +		ret = csiphy_set_clock_rates_legacy(csiphy);
>  		if (ret < 0) {
>  			regulator_bulk_disable(csiphy->num_supplies,
>  					       csiphy->supplies);
> @@ -254,7 +256,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
>  }
>  
>  /*
> - * csiphy_stream_on - Enable streaming on CSIPHY module
> + * csiphy_stream_on_legacy - Enable streaming on CSIPHY module
>   * @csiphy: CSIPHY device
>   *
>   * Helper function to enable streaming on CSIPHY module.
> @@ -262,7 +264,7 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
>   *
>   * Return 0 on success or a negative error code otherwise
>   */
> -static int csiphy_stream_on(struct csiphy_device *csiphy)
> +static int csiphy_stream_on_legacy(struct csiphy_device *csiphy)
>  {
>  	struct csiphy_config *cfg = &csiphy->cfg;
>  	s64 link_freq;
> @@ -306,11 +308,86 @@ static int csiphy_stream_on(struct csiphy_device *csiphy)
>   *
>   * Helper function to disable streaming on CSIPHY module
>   */
> -static void csiphy_stream_off(struct csiphy_device *csiphy)
> +static void csiphy_stream_off_legacy(struct csiphy_device *csiphy)
>  {
>  	csiphy->res->hw_ops->lanes_disable(csiphy, &csiphy->cfg);
>  }
>  
> +/*
> + * csiphy_stream_on - Enable streaming on CSIPHY module
> + * @csiphy: CSIPHY device
> + *
> + * Helper function to enable streaming on CSIPHY module.
> + * Main configuration of CSIPHY module is also done here.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csiphy_stream_on(struct csiphy_device *csiphy)
> +{
> +	u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
> +				csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
> +	u8 num_lanes = csiphy->cfg.csi2->lane_cfg.num_data;
> +	struct phy_configure_opts_mipi_dphy *dphy_cfg;
> +	union phy_configure_opts dphy_opts = { 0 };
> +	struct device *dev = csiphy->camss->dev;
> +	s64 link_freq;
> +	int ret;
> +
> +	dphy_cfg = &dphy_opts.mipi_dphy;
> +
> +	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
> +
> +	if (link_freq < 0) {
> +		dev_err(dev,
> +			"Cannot get CSI2 transmitter's link frequency\n");
> +		return -EINVAL;
> +	}
> +
> +	phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes, dphy_cfg);
> +
> +	phy_set_mode(csiphy->phy, PHY_MODE_MIPI_DPHY);
> +	ret = phy_configure(csiphy->phy, &dphy_opts);
> +	if (ret) {
> +		dev_err(dev, "failed to configure MIPI D-PHY\n");
> +		goto error;
> +	}
> +
> +	return phy_power_on(csiphy->phy);
> +
> +error:
> +	return ret;
> +}
> +
> +/*
> + * csiphy_stream_off - Disable streaming on CSIPHY module
> + * @csiphy: CSIPHY device
> + *
> + * Helper function to disable streaming on CSIPHY module
> + */
> +static void csiphy_stream_off(struct csiphy_device *csiphy)
> +{
> +	phy_power_off(csiphy->phy);
> +}
> +
> +/*
> + * csiphy_set_stream - Enable/disable streaming on CSIPHY module
> + * @sd: CSIPHY V4L2 subdevice
> + * @enable: Requested streaming state
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csiphy_set_stream_legacy(struct v4l2_subdev *sd, int enable)
> +{
> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
> +	int ret = 0;
> +
> +	if (enable)
> +		ret = csiphy_stream_on_legacy(csiphy);
> +	else
> +		csiphy_stream_off_legacy(csiphy);
> +
> +	return ret;
> +}
>  
>  /*
>   * csiphy_set_stream - Enable/disable streaming on CSIPHY module
> @@ -568,16 +645,16 @@ static bool csiphy_match_clock_name(const char *clock_name, const char *format,
>  }
>  
>  /*
> - * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
> + * msm_csiphy_subdev_init_legacy - Initialize CSIPHY device structure and resources
>   * @csiphy: CSIPHY device
>   * @res: CSIPHY module resources table
>   * @id: CSIPHY module id
>   *
>   * Return 0 on success or a negative error code otherwise
>   */
> -int msm_csiphy_subdev_init(struct camss *camss,
> -			   struct csiphy_device *csiphy,
> -			   const struct camss_subdev_resources *res, u8 id)
> +int msm_csiphy_subdev_init_legacy(struct camss *camss,
> +				  struct csiphy_device *csiphy,
> +				  const struct camss_subdev_resources *res, u8 id)
>  {
>  	struct device *dev = camss->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> @@ -705,6 +782,69 @@ int msm_csiphy_subdev_init(struct camss *camss,
>  	return ret;
>  }
>  
> +/*
> + * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
> + * @csiphy: CSIPHY device
> + * @res: CSIPHY module resources table
> + * @id: CSIPHY module id
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +int msm_csiphy_subdev_init(struct camss *camss,
> +			   struct csiphy_device *csiphy,
> +			   const struct camss_subdev_resources *res, u8 id)
> +{
> +	struct device *dev = camss->dev;
> +	struct of_phandle_args args;
> +	u8 combo_mode;
> +	int idx;
> +	int ret;
> +
> +	snprintf(csiphy->name, ARRAY_SIZE(csiphy->name), "csiphy%d", id);
> +
> +	idx = of_property_match_string(dev->of_node, "phy-names", csiphy->name);
> +	if (idx < 0) {
> +		dev_err(dev, "%s not found\n", csiphy->name);
> +		return idx;
> +	}
> +
> +	ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells", idx, &args);
> +	if (ret < 0) {
> +		dev_err(dev, "unable to parse phys args %s\n", csiphy->name);
> +		return ret;
> +	}
> +
> +	if (!of_device_is_available(args.np))
> +		goto put_np;
> +
> +	combo_mode = args.args[0];
> +	if (combo_mode != PHY_TYPE_DPHY) {
> +		dev_err(dev, "%s mode %d not supported\n", csiphy->name, combo_mode);
> +		ret = -ENOTSUPP;
> +		goto put_np;
> +	}
> +
> +	csiphy->phy = devm_phy_get(dev, csiphy->name);
> +	if (IS_ERR(csiphy->phy)) {
> +		ret = PTR_ERR(csiphy->phy);
> +		goto put_np;
> +	}
> +
> +	csiphy->camss = camss;
> +	csiphy->id = id;
> +	csiphy->cfg.combo_mode = combo_mode;
> +	csiphy->res = &res->csiphy;
> +
> +	ret = phy_init(csiphy->phy);
> +	if (ret)
> +		dev_err(dev, "phy %s init fail %d\n", csiphy->name, ret);
> +
> +put_np:
> +	of_node_put(args.np);
> +
> +	return ret;
> +}
> +
>  /*
>   * csiphy_link_setup - Setup CSIPHY connections
>   * @entity: Pointer to media entity structure
> @@ -739,8 +879,12 @@ static int csiphy_link_setup(struct media_entity *entity,
>  	return 0;
>  }
>  
> -static const struct v4l2_subdev_core_ops csiphy_core_ops = {
> -	.s_power = csiphy_set_power,
> +static const struct v4l2_subdev_core_ops csiphy_core_ops_legacy = {
> +	.s_power = csiphy_set_power_legacy,
> +};
> +
> +static const struct v4l2_subdev_video_ops csiphy_video_ops_legacy = {
> +	.s_stream = csiphy_set_stream_legacy,
>  };
>  
>  static const struct v4l2_subdev_video_ops csiphy_video_ops = {
> @@ -754,8 +898,13 @@ static const struct v4l2_subdev_pad_ops csiphy_pad_ops = {
>  	.set_fmt = csiphy_set_format,
>  };
>  
> +static const struct v4l2_subdev_ops csiphy_v4l2_ops_legacy = {
> +	.core = &csiphy_core_ops_legacy,
> +	.video = &csiphy_video_ops_legacy,
> +	.pad = &csiphy_pad_ops,
> +};
> +
>  static const struct v4l2_subdev_ops csiphy_v4l2_ops = {
> -	.core = &csiphy_core_ops,
>  	.video = &csiphy_video_ops,
>  	.pad = &csiphy_pad_ops,
>  };
> @@ -784,7 +933,11 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>  	struct device *dev = csiphy->camss->dev;
>  	int ret;
>  
> -	v4l2_subdev_init(sd, &csiphy_v4l2_ops);
> +	if (IS_ERR(csiphy->phy))
> +		v4l2_subdev_init(sd, &csiphy_v4l2_ops_legacy);
> +	else
> +		v4l2_subdev_init(sd, &csiphy_v4l2_ops);
> +
>  	sd->internal_ops = &csiphy_v4l2_internal_ops;
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  	snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
> @@ -823,6 +976,8 @@ int msm_csiphy_register_entity(struct csiphy_device *csiphy,
>   */
>  void msm_csiphy_unregister_entity(struct csiphy_device *csiphy)
>  {
> +	if (!IS_ERR(csiphy->phy))
> +		phy_exit(csiphy->phy);
>  	v4l2_device_unregister_subdev(&csiphy->subdev);
>  	media_entity_cleanup(&csiphy->subdev.entity);
>  }
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.h b/drivers/media/platform/qcom/camss/camss-csiphy.h
> index 2d5054819df7f..25b803c06e8bf 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.h
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.h
> @@ -12,6 +12,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/interrupt.h>
> +#include <linux/phy/phy.h>
>  #include <media/media-entity.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-mediabus.h>
> @@ -95,6 +96,7 @@ struct csiphy_device_regs {
>  
>  struct csiphy_device {
>  	struct camss *camss;
> +	struct phy *phy;
>  	u8 id;
>  	struct v4l2_subdev subdev;
>  	struct media_pad pads[MSM_CSIPHY_PADS_NUM];
> @@ -102,6 +104,7 @@ struct csiphy_device {
>  	void __iomem *base_clk_mux;
>  	u32 irq;
>  	char irq_name[30];
> +	char name[16];
>  	struct camss_clock *clock;
>  	bool *rate_set;
>  	int nclocks;
> @@ -116,6 +119,10 @@ struct csiphy_device {
>  
>  struct camss_subdev_resources;
>  
> +int msm_csiphy_subdev_init_legacy(struct camss *camss,
> +				  struct csiphy_device *csiphy,
> +				  const struct camss_subdev_resources *res, u8 id);
> +
>  int msm_csiphy_subdev_init(struct camss *camss,
>  			   struct csiphy_device *csiphy,
>  			   const struct camss_subdev_resources *res, u8 id);
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 5fb5ad87e1671..b1737b5d52160 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4450,14 +4450,35 @@ static int camss_parse_endpoint_node(struct device *dev,
>  static int camss_parse_ports(struct camss *camss)
>  {
>  	struct device *dev = camss->dev;
> +	const struct camss_resources *res = camss->res;
>  	struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
>  	int ret;
>  
>  	fwnode_graph_for_each_endpoint(fwnode, ep) {
>  		struct camss_async_subdev *csd;
> +		struct fwnode_handle *remote;
> +
> +		if (!fwnode_device_is_available(ep))
> +			continue;
> +
> +		if (res->legacy_phy) {
> +			csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
> +							      typeof(*csd));
> +		} else {
> +			/*
> +			 * For non-legacy PHY, the CSIPHY is a separate device.
> +			 * Register the remote endpoint (CSIPHY's endpoint) as
> +			 * the async subdev, not the remote port parent.
> +			 */
> +			remote = fwnode_graph_get_remote_endpoint(ep);
> +			if (!remote)
> +				continue;
> +
> +			csd = v4l2_async_nf_add_fwnode(&camss->notifier, remote,
> +						       struct camss_async_subdev);
> +			fwnode_handle_put(remote);
> +		}
>  
> -		csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
> -						      typeof(*csd));
>  		if (IS_ERR(csd)) {
>  			ret = PTR_ERR(csd);
>  			goto err_cleanup;
> @@ -4489,15 +4510,26 @@ static int camss_init_subdevices(struct camss *camss)
>  	unsigned int i;
>  	int ret;
>  
> -	for (i = 0; i < camss->res->csiphy_num; i++) {
> -		ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
> -					     &res->csiphy_res[i],
> -					     res->csiphy_res[i].csiphy.id);
> -		if (ret < 0) {
> -			dev_err(camss->dev,
> -				"Failed to init csiphy%d sub-device: %d\n",
> -				i, ret);
> -			return ret;
> +	if (!res->legacy_phy) {
> +		for (i = 0; i < camss->res->csiphy_num; i++) {
> +			ret = msm_csiphy_subdev_init(camss, &camss->csiphy[i],
> +						     &res->csiphy_res[i],
> +						     res->csiphy_res[i].csiphy.id);
> +			if (ret < 0)
> +				return ret;
> +		}
> +	} else {
> +		for (i = 0; i < camss->res->csiphy_num; i++) {
> +			ret = msm_csiphy_subdev_init_legacy(camss, &camss->csiphy[i],
> +							    &res->csiphy_res[i],
> +							    res->csiphy_res[i].csiphy.id);
> +			if (ret < 0) {
> +				dev_err(camss->dev,
> +					"Failed to init csiphy%d sub-device: %d\n",
> +					i, ret);
> +				return ret;
> +			}
> +			camss->csiphy[i].phy = ERR_PTR(-ENODEV);
>  		}
>  	}
>  
> @@ -4574,6 +4606,9 @@ static int camss_link_entities(struct camss *camss)
>  
>  	for (i = 0; i < camss->res->csiphy_num; i++) {
>  		for (j = 0; j < camss->res->csid_num; j++) {
> +			if (!camss->csiphy[i].phy)
> +				continue;
> +
>  			ret = media_create_pad_link(&camss->csiphy[i].subdev.entity,
>  						    MSM_CSIPHY_PAD_SRC,
>  						    &camss->csid[j].subdev.entity,
> @@ -4683,6 +4718,9 @@ static int camss_register_entities(struct camss *camss)
>  	int ret;
>  
>  	for (i = 0; i < camss->res->csiphy_num; i++) {
> +		if (!camss->csiphy[i].phy)
> +			continue;
> +
>  		ret = msm_csiphy_register_entity(&camss->csiphy[i],
>  						 &camss->v4l2_dev);
>  		if (ret < 0) {
> @@ -4738,8 +4776,10 @@ static int camss_register_entities(struct camss *camss)
>  
>  	i = camss->res->csiphy_num;
>  err_reg_csiphy:
> -	for (i--; i >= 0; i--)
> -		msm_csiphy_unregister_entity(&camss->csiphy[i]);
> +	for (i--; i >= 0; i--) {
> +		if (camss->csiphy[i].phy)
> +			msm_csiphy_unregister_entity(&camss->csiphy[i]);
> +	}
>  
>  	return ret;
>  }
> @@ -4754,8 +4794,10 @@ static void camss_unregister_entities(struct camss *camss)
>  {
>  	unsigned int i;
>  
> -	for (i = 0; i < camss->res->csiphy_num; i++)
> -		msm_csiphy_unregister_entity(&camss->csiphy[i]);
> +	for (i = 0; i < camss->res->csiphy_num; i++) {
> +		if (camss->csiphy[i].phy)
> +			msm_csiphy_unregister_entity(&camss->csiphy[i]);
> +	}
>  
>  	for (i = 0; i < camss->res->csid_num; i++)
>  		msm_csid_unregister_entity(&camss->csid[i]);