[v3,12/15] media: qcom: camss: Fix support for setting CSIPHY clock name csiphyX

  Several of our upstream and soon-to-be upstream SoC CAMSS dtsi declare
csiphyX as opposed to the older clock name csiX_phy.

Right now the CAMSS code will fail to set the csiphyX clock even if we have
declared it in our list of clocks. For sdm845 and sm8250 we appear to "get
away" with this error, however on sc8280xp we don't.

The right approach here is to set the clock when it is declared. If a SoC
doesn't require or a SoC driver implementer doesn't think we need, then the
clock ought to simply be omitted from the clock list.

Include csiphyX in the set of permissible strings which will subsequently
lead to the csiphyX clock being set during csiphy_set_clock_rates() phase.

sdm845 and sm8250 will work with the code as-is so I've omitted this from a
suggested Fixes list.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/media/platform/qcom/camss/camss-csiphy.c | 4 ++++
 1 file changed, 4 insertions(+)
  

On 23.08.2023 12:44, Bryan O'Donoghue wrote:
> Several of our upstream and soon-to-be upstream SoC CAMSS dtsi declare
> csiphyX as opposed to the older clock name csiX_phy.
This only reinforces my point about adding like csiphy_clks or so

Konrad
> 
> Right now the CAMSS code will fail to set the csiphyX clock even if we have
> declared it in our list of clocks. For sdm845 and sm8250 we appear to "get
> away" with this error, however on sc8280xp we don't.
> 
> The right approach here is to set the clock when it is declared. If a SoC
> doesn't require or a SoC driver implementer doesn't think we need, then the
> clock ought to simply be omitted from the clock list.
> 
> Include csiphyX in the set of permissible strings which will subsequently
> lead to the csiphyX clock being set during csiphy_set_clock_rates() phase.
> 
> sdm845 and sm8250 will work with the code as-is so I've omitted this from a
> suggested Fixes list.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/camss/camss-csiphy.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index baf78c525fbfc..d9c751f457703 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
> @@ -687,6 +687,10 @@ int msm_csiphy_subdev_init(struct camss *camss,
>  				if (csiphy->rate_set[i])
>  					break;
>  			}
> +
> +			csiphy->rate_set[i] = csiphy_match_clock_name(clock->name, "csiphy%d", k);
> +			if (csiphy->rate_set[i])
> +				break;
>  		}
>  	}
>
  

On 26/08/2023 11:13, Konrad Dybcio wrote:
> On 23.08.2023 12:44, Bryan O'Donoghue wrote:
>> Several of our upstream and soon-to-be upstream SoC CAMSS dtsi declare
>> csiphyX as opposed to the older clock name csiX_phy.
> This only reinforces my point about adding like csiphy_clks or so
> 
> Konrad

I really don't understand your point. Could you please restate it ?

---
bod
  

On 26.08.2023 14:08, Bryan O'Donoghue wrote:
> On 26/08/2023 11:13, Konrad Dybcio wrote:
>> On 23.08.2023 12:44, Bryan O'Donoghue wrote:
>>> Several of our upstream and soon-to-be upstream SoC CAMSS dtsi declare
>>> csiphyX as opposed to the older clock name csiX_phy.
>> This only reinforces my point about adding like csiphy_clks or so
>>
>> Konrad
> 
> I really don't understand your point. Could you please restate it ?
If we categorized the clocks at probe time (these ones go to csiphy, these
ones go to vfe or whatever), name matching like this could be avoided

Konrad
  

On 26/08/2023 13:12, Konrad Dybcio wrote:
>> I really don't understand your point. Could you please restate it ?
> If we categorized the clocks at probe time (these ones go to csiphy, these
> ones go to vfe or whatever), name matching like this could be avoided
> 
> Konrad

Yes, I like this idea.

I'd like to make that into a separate series. So I'd like to address 
your concern on the size of the string in the lookup and then punt the 
clock story over to another series since it will involved churning 
though a fair chunk of code, yaml and dtsi.

---
bod
  

On 4.09.2023 21:11, Bryan O'Donoghue wrote:
> On 26/08/2023 13:12, Konrad Dybcio wrote:
>>> I really don't understand your point. Could you please restate it ?
>> If we categorized the clocks at probe time (these ones go to csiphy, these
>> ones go to vfe or whatever), name matching like this could be avoided
>>
>> Konrad
> 
> Yes, I like this idea.
> 
> I'd like to make that into a separate series. So I'd like to address your concern on the size of the string in the lookup and then punt the clock story over to another series since it will involved churning though a fair chunk of code, yaml and dtsi.
I can only think of code changes, but fine, this series is large as-is.

Konrad
  

On 04/09/2023 20:32, Konrad Dybcio wrote:
> On 4.09.2023 21:11, Bryan O'Donoghue wrote:
>> On 26/08/2023 13:12, Konrad Dybcio wrote:
>>>> I really don't understand your point. Could you please restate it ?
>>> If we categorized the clocks at probe time (these ones go to csiphy, these
>>> ones go to vfe or whatever), name matching like this could be avoided
>>>
>>> Konrad
>>
>> Yes, I like this idea.
>>
>> I'd like to make that into a separate series. So I'd like to address your concern on the size of the string in the lookup and then punt the clock story over to another series since it will involved churning though a fair chunk of code, yaml and dtsi.
> I can only think of code changes, but fine, this series is large as-is.
> 
> Konrad

Agreed.

I pinky-swear to do this series..

---
bod