| Message ID | 20250422-sc8180x-camcc-support-v1-0-691614d13f06@quicinc.com |
|---|---|
| Headers | show |
| Series | clk: qcom: Add camera clock controller support for sc8180x | expand |
On 4/24/25 12:38 PM, Satya Priya Kakitapalli wrote: > > On 4/22/2025 5:11 PM, Bryan O'Donoghue wrote: >> On 22/04/2025 06:42, Satya Priya Kakitapalli wrote: >>> Add device tree bindings for the camera clock controller on >>> Qualcomm SC8180X platform. >>> >>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >>> --- [...] >>> + required-opps: >>> + maxItems: 1 >>> + description: >>> + A phandle to an OPP node describing required MMCX performance point. >>> + >>> +allOf: >>> + - $ref: qcom,gcc.yaml# >> >> A suspicious lack of clock depends here. No AHB clock ? No dependency on gcc ? >> >> You call out the gcc above. >> >> Could you please recheck your list of clock dependencies. > > The dependent GCC clocks are marked always on from gcc probe, hence did not mention the dependency here. Let's do what was done on x1e80100 - describe the AHB clock in CAMCC bindings regardless of how we handle it. This way the DT represents the real hw dependency, but the OS takes steps to get them out of the way (and then ignores the GCC_CAMERA_AHB_CLK entry because the clock is never registered with GCC) Konrad
On 25/04/2025 10:35, Konrad Dybcio wrote: >> The dependent GCC clocks are marked always on from gcc probe, hence did not mention the dependency here. > Let's do what was done on x1e80100 - describe the AHB clock in CAMCC > bindings regardless of how we handle it. > > This way the DT represents the real hw dependency, but the OS takes steps > to get them out of the way (and then ignores the GCC_CAMERA_AHB_CLK entry > because the clock is never registered with GCC) Ah yes, this is an always-on clock isn't it ? But in principle I agree, the DTS should just describe the hw as-is. --- bod
On 25/04/2025 11:06, Bryan O'Donoghue wrote: > On 25/04/2025 10:35, Konrad Dybcio wrote: >>> The dependent GCC clocks are marked always on from gcc probe, hence >>> did not mention the dependency here. >> Let's do what was done on x1e80100 - describe the AHB clock in CAMCC >> bindings regardless of how we handle it. >> >> This way the DT represents the real hw dependency, but the OS takes steps >> to get them out of the way (and then ignores the GCC_CAMERA_AHB_CLK entry >> because the clock is never registered with GCC) > > Ah yes, this is an always-on clock isn't it ? > > But in principle I agree, the DTS should just describe the hw as-is. > > --- > bod Pleasantly surprised to find that's what we've done for x1e camcc 20250314-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v6-3-edcb2cfc3122@linaro.org --- bod
On 4/22/25 08:42, Satya Priya Kakitapalli wrote: > Add support for the camera clock controller for camera clients to > be able to request for camcc clocks on SC8180X platform. > > Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> > --- > drivers/clk/qcom/Kconfig | 10 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/camcc-sc8180x.c | 2896 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 2907 insertions(+) > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 7d5dac26b244bfe785370033ad8ba49876d6627d..42b64e34b3fcc4bae7c559f34a34f9452307ae9a 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -900,6 +900,16 @@ config SDX_GCC_75 > Say Y if you want to use peripheral devices such as UART, > SPI, I2C, USB, SD/eMMC, PCIe etc. > > +config SC_CAMCC_8180X > + tristate "SC8180X Camera Clock Controller" > + depends on ARM64 || COMPILE_TEST > + select SC_GCC_8180X > + help > + Support for the camera clock controller on Qualcomm Technologies, Inc > + SC8180X devices. > + Say Y if you want to support camera devices and functionality such as > + capturing pictures. > + > config SM_CAMCC_4450 > tristate "SM4450 Camera Clock Controller" > depends on ARM64 || COMPILE_TEST Please add a new config section preserving the alphanumerical order. The new section should be placed between SC_CAMCC_7280 and SC_CAMCC_8280XP, like it's correctly done for the Makefile below. After fixing it: Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> -- Best wishes, Vladimir
On 4/22/25 08:42, Satya Priya Kakitapalli wrote: > Add device tree bindings for the camera clock controller on > Qualcomm SC8180X platform. > > Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> > --- > .../bindings/clock/qcom,sc8180x-camcc.yaml | 65 ++++++++ > include/dt-bindings/clock/qcom,sc8180x-camcc.h | 181 +++++++++++++++++++++ > 2 files changed, 246 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..b17f40ee53a3002b2942869d60773dbecd764134 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/qcom,sc8180x-camcc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Camera Clock & Reset Controller on SC8180X > + > +maintainers: > + - Satya Priya Kakitapalli <quic_skakitap@quicinc.com> > + > +description: | > + Qualcomm camera clock control module provides the clocks, resets and > + power domains on SC8180X. > + > + See also: include/dt-bindings/clock/qcom,sc8180x-camcc.h > + > +properties: > + compatible: > + const: qcom,sc8180x-camcc > + > + clocks: > + items: > + - description: Board XO source From sc8180x_rpmh_clocks[] in clk/qcom/clk-rpmh.c I get that there is RPMH_CXO_CLK_A clock also, shall it be added to this list then? If yes, and taking into account Konrad's ask for GCC_CAMERA_AHB_CLK, it implies that the new dt bindings can be omitted, instead please consider to add the 'qcom,sc8180x-camcc' compatible into qcom,sa8775p-camcc.yaml. However still there is a difference, qcom,sa8775p-camcc and qcom,qcs8300-camcc does not contain 'required-opps' property, it might be an omission over there though, please double check it. The ultimate goal would be to get a shorter list of different camcc dt bindings. > + - description: Sleep clock source > + > + power-domains: > + maxItems: 1 > + description: > + A phandle and PM domain specifier for the MMCX power domain. > + > + required-opps: > + maxItems: 1 > + description: > + A phandle to an OPP node describing required MMCX performance point. > + > +allOf: > + - $ref: qcom,gcc.yaml# > + > +required: > + - compatible > + - clocks > + - power-domains > + - required-opps > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sc8180x.h> > + #include <dt-bindings/clock/qcom,rpmh.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + clock-controller@ad00000 { > + compatible = "qcom,sc8180x-camcc"; > + reg = <0x0ad00000 0x20000>; > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&sleep_clk>; > + power-domains = <&rpmhpd SC8180X_MMCX>; > + required-opps = <&rpmhpd_opp_low_svs>; > + > + #clock-cells = <1>; > + #reset-cells = <1>; > + #power-domain-cells = <1>; > + }; > +... -- Best wishes, Vladimir
On 4/26/25 4:03 PM, Vladimir Zapolskiy wrote: > On 4/22/25 08:42, Satya Priya Kakitapalli wrote: >> Add device tree bindings for the camera clock controller on >> Qualcomm SC8180X platform. >> >> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >> --- >> .../bindings/clock/qcom,sc8180x-camcc.yaml | 65 ++++++++ >> include/dt-bindings/clock/qcom,sc8180x-camcc.h | 181 +++++++++++++++++++++ >> 2 files changed, 246 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..b17f40ee53a3002b2942869d60773dbecd764134 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml >> @@ -0,0 +1,65 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/clock/qcom,sc8180x-camcc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm Camera Clock & Reset Controller on SC8180X >> + >> +maintainers: >> + - Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >> + >> +description: | >> + Qualcomm camera clock control module provides the clocks, resets and >> + power domains on SC8180X. >> + >> + See also: include/dt-bindings/clock/qcom,sc8180x-camcc.h >> + >> +properties: >> + compatible: >> + const: qcom,sc8180x-camcc >> + >> + clocks: >> + items: >> + - description: Board XO source > > From sc8180x_rpmh_clocks[] in clk/qcom/clk-rpmh.c I get that there is > RPMH_CXO_CLK_A clock also, shall it be added to this list then? _AO only makes sense for CPU-XYZ clocks which (almost?) exclusively reside in gcc, and most of the time they aren't even necessary to describe in Linux I'm not sure there's anything in CAM_CC that would benefit from it XO_A is in the end the same physical clock as XO (or at least used to be) except it places a vote in RPMh such that the harwdare takes care of gating it upon cpuss suspend entry > > If yes, and taking into account Konrad's ask for GCC_CAMERA_AHB_CLK, it > implies that the new dt bindings can be omitted, instead please consider > to add the 'qcom,sc8180x-camcc' compatible into qcom,sa8775p-camcc.yaml. > > However still there is a difference, qcom,sa8775p-camcc and qcom,qcs8300-camcc > does not contain 'required-opps' property, it might be an omission over > there though, please double check it. The ultimate goal would be to get > a shorter list of different camcc dt bindings. for required-opps see: https://lore.kernel.org/linux-arm-msm/44dad3b5-ea3d-47db-8aca-8f67294fced9@quicinc.com/ Konrad
On 4/26/2025 6:54 PM, Vladimir Zapolskiy wrote: > On 4/22/25 08:42, Satya Priya Kakitapalli wrote: >> Add support for the camera clock controller for camera clients to >> be able to request for camcc clocks on SC8180X platform. >> >> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >> --- >> drivers/clk/qcom/Kconfig | 10 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/camcc-sc8180x.c | 2896 >> ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 2907 insertions(+) >> >> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >> index >> 7d5dac26b244bfe785370033ad8ba49876d6627d..42b64e34b3fcc4bae7c559f34a34f9452307ae9a >> 100644 >> --- a/drivers/clk/qcom/Kconfig >> +++ b/drivers/clk/qcom/Kconfig >> @@ -900,6 +900,16 @@ config SDX_GCC_75 >> Say Y if you want to use peripheral devices such as UART, >> SPI, I2C, USB, SD/eMMC, PCIe etc. >> +config SC_CAMCC_8180X >> + tristate "SC8180X Camera Clock Controller" >> + depends on ARM64 || COMPILE_TEST >> + select SC_GCC_8180X >> + help >> + Support for the camera clock controller on Qualcomm >> Technologies, Inc >> + SC8180X devices. >> + Say Y if you want to support camera devices and functionality >> such as >> + capturing pictures. >> + >> config SM_CAMCC_4450 >> tristate "SM4450 Camera Clock Controller" >> depends on ARM64 || COMPILE_TEST > > Please add a new config section preserving the alphanumerical order. > > The new section should be placed between SC_CAMCC_7280 and > SC_CAMCC_8280XP, > like it's correctly done for the Makefile below. > Sure, will fix it. Thanks. > After fixing it: > > Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > > -- > Best wishes, > Vladimir
On 4/24/2025 4:08 PM, Satya Priya Kakitapalli wrote: > > On 4/22/2025 5:11 PM, Bryan O'Donoghue wrote: >> On 22/04/2025 06:42, Satya Priya Kakitapalli wrote: >>> Add device tree bindings for the camera clock controller on >>> Qualcomm SC8180X platform. >>> >>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >>> --- >>> .../bindings/clock/qcom,sc8180x-camcc.yaml | 65 ++++++++ >>> include/dt-bindings/clock/qcom,sc8180x-camcc.h | 181 >>> +++++++++++++++++++++ >>> 2 files changed, 246 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml >>> b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml >>> new file mode 100644 >>> index >>> 0000000000000000000000000000000000000000..b17f40ee53a3002b2942869d60773dbecd764134 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml >>> @@ -0,0 +1,65 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/clock/qcom,sc8180x-camcc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Qualcomm Camera Clock & Reset Controller on SC8180X >>> + >>> +maintainers: >>> + - Satya Priya Kakitapalli <quic_skakitap@quicinc.com> >>> + >>> +description: | >> >> You can drop the "|" >> > > okay. > > I tried removing this, but it is throwing below error, in the line "See also: include/dt-bindings/clock/qcom,sc8180x-camcc.h" because of usage of a colon there. Error: Documentation/devicetree/bindings/clock/qcom,sc8180x-camcc.yaml:16:12: mapping values are not allowed in this context As we are following the same format in all other Qualcomm bindings, I'll keep the "|" and "See also:" section as is. >>> + Qualcomm camera clock control module provides the clocks, resets and >>> + power domains on SC8180X. >>> + >>> + See also: include/dt-bindings/clock/qcom,sc8180x-camcc.h >>> + >>> +properties: >>> + compatible: >>> + const: qcom,sc8180x-camcc >>> + >>> + clocks: >>> + items: >>> + - description: Board XO source >>> + - description: Sleep clock source >> >> Missing clock-names >> > > Since we are using DT based indexing method, clock names are not > required. > > >> A suspicious lack of clock depends here. No AHB clock ?> + >>> + power-domains: >>> + maxItems: 1 >>> + description: >>> + A phandle and PM domain specifier for the MMCX power domain. >>> + >>> + required-opps: >>> + maxItems: 1 >>> + description: >>> + A phandle to an OPP node describing required MMCX performance >>> point. >>> + >>> +allOf: >>> + - $ref: qcom,gcc.yaml# >> >> A suspicious lack of clock depends here. No AHB clock ? No dependency >> on gcc ? >> >> You call out the gcc above. >> >> Could you please recheck your list of clock dependencies. > > The dependent GCC clocks are marked always on from gcc probe, hence > did not mention the dependency here. > > >> >> --- >> bod >
This series adds support for camera clock controller base driver, bindings and DT support on sc8180x platform. Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com> --- Satya Priya Kakitapalli (3): dt-bindings: clock: Add Qualcomm SC8180X Camera clock controller clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver arm64: dts: qcom: Add camera clock controller for sc8180x .../bindings/clock/qcom,sc8180x-camcc.yaml | 65 + arch/arm64/boot/dts/qcom/sc8180x.dtsi | 13 + drivers/clk/qcom/Kconfig | 10 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/camcc-sc8180x.c | 2896 ++++++++++++++++++++ include/dt-bindings/clock/qcom,sc8180x-camcc.h | 181 ++ 6 files changed, 3166 insertions(+) --- base-commit: bc8aa6cdadcc00862f2b5720e5de2e17f696a081 change-id: 20250422-sc8180x-camcc-support-9a82507d2a39 Best regards,