| Message ID | 20230823104444.1954663-13-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: qcom: camss: Add parameter passing to remove several outstanding bugs | expand |
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
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; } }
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(+)