| Message ID | 20241209-camss-dphy-v1-2-5f1b6f25ed92@fairphone.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Some small preparations around CAMSS D-PHY / C-PHY support | expand |
On 09/12/2024 12:01, Luca Weiss wrote: > Currently the Qualcomm CAMSS driver only supports D-PHY while the > hardware on most SoCs also supports C-PHY. Until this support is added, > check for D-PHY to make it somewhat explicit that C-PHY won't work. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, > if (ret) > return ret; > > + /* > + * Most SoCs support both D-PHY and C-PHY standards, but currently only > + * D-PHY is supported in the driver. > + */ > + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { > + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); > + return -EINVAL; > + } > + > csd->interface.csiphy_id = vep.base.port; > > mipi_csi2 = &vep.bus.mipi_csi2; > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 12/9/24 14:32, Bryan O'Donoghue wrote: > On 09/12/2024 12:01, Luca Weiss wrote: >> Currently the Qualcomm CAMSS driver only supports D-PHY while the >> hardware on most SoCs also supports C-PHY. Until this support is added, >> check for D-PHY to make it somewhat explicit that C-PHY won't work. >> >> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >> --- >> drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c >> index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 >> --- a/drivers/media/platform/qcom/camss/camss.c >> +++ b/drivers/media/platform/qcom/camss/camss.c >> @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, >> if (ret) >> return ret; >> >> + /* >> + * Most SoCs support both D-PHY and C-PHY standards, but currently only >> + * D-PHY is supported in the driver. >> + */ >> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { >> + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); >> + return -EINVAL; >> + } >> + Looks like it would break all old board dtbs, which is not just bad, but NAK. V4L2_MBUS_UNKNOWN shall be properly handled without the risk of regressions. >> csd->interface.csiphy_id = vep.base.port; >> >> mipi_csi2 = &vep.bus.mipi_csi2; >> > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > -- Best wishes, Vladimir
On Fri Dec 13, 2024 at 12:02 PM CET, Vladimir Zapolskiy wrote: > On 12/9/24 14:32, Bryan O'Donoghue wrote: > > On 09/12/2024 12:01, Luca Weiss wrote: > >> Currently the Qualcomm CAMSS driver only supports D-PHY while the > >> hardware on most SoCs also supports C-PHY. Until this support is added, > >> check for D-PHY to make it somewhat explicit that C-PHY won't work. > >> > >> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >> --- > >> drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > >> index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 > >> --- a/drivers/media/platform/qcom/camss/camss.c > >> +++ b/drivers/media/platform/qcom/camss/camss.c > >> @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, > >> if (ret) > >> return ret; > >> > >> + /* > >> + * Most SoCs support both D-PHY and C-PHY standards, but currently only > >> + * D-PHY is supported in the driver. > >> + */ > >> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { > >> + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); > >> + return -EINVAL; > >> + } > >> + > > Looks like it would break all old board dtbs, which is not just bad, but NAK. > > V4L2_MBUS_UNKNOWN shall be properly handled without the risk of regressions. Please see drivers/media/v4l2-core/v4l2-fwnode.c around line 218. The code there sets bus_type if it's UNKNOWN if (bus_type == V4L2_MBUS_UNKNOWN) vep->bus_type = V4L2_MBUS_CSI2_DPHY; So setting "bus-type" in dt is not necessary, even if it makes things more explicit from dt side. I don't think we'll ever get UNKNOWN here in camss. Regards Luca > > >> csd->interface.csiphy_id = vep.base.port; > >> > >> mipi_csi2 = &vep.bus.mipi_csi2; > >> > > > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > > > -- > Best wishes, > Vladimir
On 12/13/24 13:22, Luca Weiss wrote: > On Fri Dec 13, 2024 at 12:02 PM CET, Vladimir Zapolskiy wrote: >> On 12/9/24 14:32, Bryan O'Donoghue wrote: >>> On 09/12/2024 12:01, Luca Weiss wrote: >>>> Currently the Qualcomm CAMSS driver only supports D-PHY while the >>>> hardware on most SoCs also supports C-PHY. Until this support is added, >>>> check for D-PHY to make it somewhat explicit that C-PHY won't work. >>>> >>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>> --- >>>> drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c >>>> index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 >>>> --- a/drivers/media/platform/qcom/camss/camss.c >>>> +++ b/drivers/media/platform/qcom/camss/camss.c >>>> @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, >>>> if (ret) >>>> return ret; >>>> >>>> + /* >>>> + * Most SoCs support both D-PHY and C-PHY standards, but currently only >>>> + * D-PHY is supported in the driver. >>>> + */ >>>> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { >>>> + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); >>>> + return -EINVAL; >>>> + } >>>> + >> >> Looks like it would break all old board dtbs, which is not just bad, but NAK. >> >> V4L2_MBUS_UNKNOWN shall be properly handled without the risk of regressions. > > Please see drivers/media/v4l2-core/v4l2-fwnode.c around line 218. > The code there sets bus_type if it's UNKNOWN > > if (bus_type == V4L2_MBUS_UNKNOWN) > vep->bus_type = V4L2_MBUS_CSI2_DPHY; > > So setting "bus-type" in dt is not necessary, even if it makes things > more explicit from dt side. I don't think we'll ever get UNKNOWN here in > camss. Thank you for pointing it out, I haven't tested the change yet, but hopefully I will find time today to do it later on, it should exclude any doubts. -- Best wishes, Vladimir
Hi Luca. On 12/9/24 14:01, Luca Weiss wrote: > Currently the Qualcomm CAMSS driver only supports D-PHY while the > hardware on most SoCs also supports C-PHY. Until this support is added, > check for D-PHY to make it somewhat explicit that C-PHY won't work. > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > --- > drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c > index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 > --- a/drivers/media/platform/qcom/camss/camss.c > +++ b/drivers/media/platform/qcom/camss/camss.c > @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, > if (ret) > return ret; > > + /* > + * Most SoCs support both D-PHY and C-PHY standards, but currently only > + * D-PHY is supported in the driver. > + */ > + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { > + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); > + return -EINVAL; > + } > + > csd->interface.csiphy_id = vep.base.port; > > mipi_csi2 = &vep.bus.mipi_csi2; My cautious worries were futile, the change works as expected and the regression testing on RB5 is passed: ===== begin parsing endpoint /soc@0/camss@ac6a000/ports/port@2/endpoint fwnode video bus type not specified (0), mbus type not specified (0) lane 0 position 0 lane 1 position 1 lane 2 position 2 lane 3 position 3 clock lane position 7 no lane polarities defined, assuming not inverted assuming media bus type MIPI CSI-2 D-PHY (5) ===== end parsing endpoint /soc@0/camss@ac6a000/ports/port@2/endpoint Tested-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> -- Best wishes, Vladimir
diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c index 9fb31f4c18adee886cd0bcf84438a8f27635e07f..b99af35074cdf6fa794a0d2f0d54ecf12ac354d9 100644 --- a/drivers/media/platform/qcom/camss/camss.c +++ b/drivers/media/platform/qcom/camss/camss.c @@ -1855,6 +1855,15 @@ static int camss_of_parse_endpoint_node(struct device *dev, if (ret) return ret; + /* + * Most SoCs support both D-PHY and C-PHY standards, but currently only + * D-PHY is supported in the driver. + */ + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) { + dev_err(dev, "Unsupported bus type %d\n", vep.bus_type); + return -EINVAL; + } + csd->interface.csiphy_id = vep.base.port; mipi_csi2 = &vep.bus.mipi_csi2;
Currently the Qualcomm CAMSS driver only supports D-PHY while the hardware on most SoCs also supports C-PHY. Until this support is added, check for D-PHY to make it somewhat explicit that C-PHY won't work. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- drivers/media/platform/qcom/camss/camss.c | 9 +++++++++ 1 file changed, 9 insertions(+)