| Message ID | 20241024-qcs8300-mm-patches-v2-0-76c905060d0a@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support for GPUCC, CAMCC and VIDEOCC on Qualcomm QCS8300 platform | expand |
On 10/28/2024 12:35 PM, Krzysztof Kozlowski wrote: > On 28/10/2024 06:15, Imran Shaik wrote: >> >> >> On 10/26/2024 5:50 PM, Krzysztof Kozlowski wrote: >>> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >>> >>> IIUC, these clocks are not valid for SA8775p. How do we deal with such >>> cases for other Qualcomm SoCs? >>> >> >> These newly added clocks are not applicable to SA8755P. In the >> gpucc-sa8775p driver, these clocks are marked to NULL for the SA8755P, >> ensuring they are not registered to the CCF. > > I meant bindings. And existing practice. > In the bindings, the same approach is followed in other Qualcomm SoCs as well, where additional clocks are added to the existing identical SoC’s bindings. https://lore.kernel.org/r/20240818204348.197788-2-danila@jiaxyga.com Thanks, Imran > Best regards, > Krzysztof >
On 29/10/2024 10:23, Imran Shaik wrote: > > > On 10/28/2024 12:35 PM, Krzysztof Kozlowski wrote: >> On 28/10/2024 06:15, Imran Shaik wrote: >>> >>> >>> On 10/26/2024 5:50 PM, Krzysztof Kozlowski wrote: >>>> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>>>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>>>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>>>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >>>> >>>> IIUC, these clocks are not valid for SA8775p. How do we deal with such >>>> cases for other Qualcomm SoCs? >>>> >>> >>> These newly added clocks are not applicable to SA8755P. In the >>> gpucc-sa8775p driver, these clocks are marked to NULL for the SA8755P, >>> ensuring they are not registered to the CCF. >> >> I meant bindings. And existing practice. >> > > In the bindings, the same approach is followed in other Qualcomm SoCs as > well, where additional clocks are added to the existing identical SoC’s > bindings. > > https://lore.kernel.org/r/20240818204348.197788-2-danila@jiaxyga.com Exactly, defines are very different, so no, it is not the same approach. Best regards, Krzysztof
On 30/10/2024 07:59, Imran Shaik wrote: > > > On 10/29/2024 3:06 PM, Krzysztof Kozlowski wrote: >> On 29/10/2024 10:23, Imran Shaik wrote: >>> >>> >>> On 10/28/2024 12:35 PM, Krzysztof Kozlowski wrote: >>>> On 28/10/2024 06:15, Imran Shaik wrote: >>>>> >>>>> >>>>> On 10/26/2024 5:50 PM, Krzysztof Kozlowski wrote: >>>>>> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>>>>>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>>>>>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>>>>>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >>>>>> >>>>>> IIUC, these clocks are not valid for SA8775p. How do we deal with such >>>>>> cases for other Qualcomm SoCs? >>>>>> >>>>> >>>>> These newly added clocks are not applicable to SA8755P. In the >>>>> gpucc-sa8775p driver, these clocks are marked to NULL for the SA8755P, >>>>> ensuring they are not registered to the CCF. >>>> >>>> I meant bindings. And existing practice. >>>> >>> >>> In the bindings, the same approach is followed in other Qualcomm SoCs as >>> well, where additional clocks are added to the existing identical SoC’s >>> bindings. >>> >>> https://lore.kernel.org/r/20240818204348.197788-2-danila@jiaxyga.com >> >> Exactly, defines are very different, so no, it is not the same approach. >> > > I believe the QCS8300 approach is same as that of SM8475. In the SM8475 > SoC, GPLL2 and GPLL3 are the additional clock bindings compared to the > SM8450. Similarly, in the QCS8300, the GPU_CC_*_ACCU_SHIFT_CLK clock > bindings are additional to the SA8775P. > > We are also following this approach across all SoCs in the downstream > msm-kernel as well. > > Please let me know if I am missing anything here. Not sure, please take the same approach as SM8475, not a different one. Best regards, Krzysztof
On 30/10/2024 11:14, Imran Shaik wrote: > > > On 10/30/2024 1:00 PM, Krzysztof Kozlowski wrote: >> On 30/10/2024 07:59, Imran Shaik wrote: >>> >>> >>> On 10/29/2024 3:06 PM, Krzysztof Kozlowski wrote: >>>> On 29/10/2024 10:23, Imran Shaik wrote: >>>>> >>>>> >>>>> On 10/28/2024 12:35 PM, Krzysztof Kozlowski wrote: >>>>>> On 28/10/2024 06:15, Imran Shaik wrote: >>>>>>> >>>>>>> >>>>>>> On 10/26/2024 5:50 PM, Krzysztof Kozlowski wrote: >>>>>>>> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>>>>>>>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>>>>>>>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>>>>>>>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >>>>>>>> >>>>>>>> IIUC, these clocks are not valid for SA8775p. How do we deal with such >>>>>>>> cases for other Qualcomm SoCs? >>>>>>>> >>>>>>> >>>>>>> These newly added clocks are not applicable to SA8755P. In the >>>>>>> gpucc-sa8775p driver, these clocks are marked to NULL for the SA8755P, >>>>>>> ensuring they are not registered to the CCF. >>>>>> >>>>>> I meant bindings. And existing practice. >>>>>> >>>>> >>>>> In the bindings, the same approach is followed in other Qualcomm SoCs as >>>>> well, where additional clocks are added to the existing identical SoC’s >>>>> bindings. >>>>> >>>>> https://lore.kernel.org/r/20240818204348.197788-2-danila@jiaxyga.com >>>> >>>> Exactly, defines are very different, so no, it is not the same approach. >>>> >>> >>> I believe the QCS8300 approach is same as that of SM8475. In the SM8475 >>> SoC, GPLL2 and GPLL3 are the additional clock bindings compared to the >>> SM8450. Similarly, in the QCS8300, the GPU_CC_*_ACCU_SHIFT_CLK clock >>> bindings are additional to the SA8775P. >>> >>> We are also following this approach across all SoCs in the downstream >>> msm-kernel as well. >>> >>> Please let me know if I am missing anything here. >> >> Not sure, please take the same approach as SM8475, not a different one. >> > > Yes, it is the same approach as SM8475. I already said: "Exactly, defines are very different, so no, it is not the same approach." and this discussion leads nowhere. Don't answer with useless responses just so reviewer will go away. NAK. I am going away. Best regards, Krzysztof
On 10/26/24 15:20, Krzysztof Kozlowski wrote: > On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >> QCS8300 has few additional clocks and minor differences. Hence, reuse >> SA8775P gpucc bindings and add additional clocks required for QCS8300. > > IIUC, these clocks are not valid for SA8775p. How do we deal with such > cases for other Qualcomm SoCs? > It always possible to add a new platform specific header file and include the old one. For reference see commit a6a61b9701d1 ("dt-bindings: clock: qcom: Add SM8650 video clock controller"), I believe that's the preferred way of adding new platform clocks whenever technically possible. -- Best wishes, Vladimir
On 30/10/2024 12:23, Dmitry Baryshkov wrote: > On Wed, Oct 30, 2024 at 08:30:59AM +0100, Krzysztof Kozlowski wrote: >> On 30/10/2024 07:59, Imran Shaik wrote: >>> >>> >>> On 10/29/2024 3:06 PM, Krzysztof Kozlowski wrote: >>>> On 29/10/2024 10:23, Imran Shaik wrote: >>>>> >>>>> >>>>> On 10/28/2024 12:35 PM, Krzysztof Kozlowski wrote: >>>>>> On 28/10/2024 06:15, Imran Shaik wrote: >>>>>>> >>>>>>> >>>>>>> On 10/26/2024 5:50 PM, Krzysztof Kozlowski wrote: >>>>>>>> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>>>>>>>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>>>>>>>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>>>>>>>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >>>>>>>> >>>>>>>> IIUC, these clocks are not valid for SA8775p. How do we deal with such >>>>>>>> cases for other Qualcomm SoCs? >>>>>>>> >>>>>>> >>>>>>> These newly added clocks are not applicable to SA8755P. In the >>>>>>> gpucc-sa8775p driver, these clocks are marked to NULL for the SA8755P, >>>>>>> ensuring they are not registered to the CCF. >>>>>> >>>>>> I meant bindings. And existing practice. >>>>>> >>>>> >>>>> In the bindings, the same approach is followed in other Qualcomm SoCs as >>>>> well, where additional clocks are added to the existing identical SoC’s >>>>> bindings. >>>>> >>>>> https://lore.kernel.org/r/20240818204348.197788-2-danila@jiaxyga.com >>>> >>>> Exactly, defines are very different, so no, it is not the same approach. >>>> >>> >>> I believe the QCS8300 approach is same as that of SM8475. In the SM8475 >>> SoC, GPLL2 and GPLL3 are the additional clock bindings compared to the >>> SM8450. Similarly, in the QCS8300, the GPU_CC_*_ACCU_SHIFT_CLK clock >>> bindings are additional to the SA8775P. >>> >>> We are also following this approach across all SoCs in the downstream >>> msm-kernel as well. >>> >>> Please let me know if I am missing anything here. >> >> Not sure, please take the same approach as SM8475, not a different one. > > Just for my understanding, are you proposing to prefix the > platform-specific defines with platform name (like it was done for > SM8475)? Yes. Maybe SM8475 did something more, so let's take similar approach. Best regards, Krzysztof
On 10/30/2024 4:29 PM, Vladimir Zapolskiy wrote: > On 10/26/24 15:20, Krzysztof Kozlowski wrote: >> On Thu, Oct 24, 2024 at 07:01:14PM +0530, Imran Shaik wrote: >>> The QCS8300 GPU clock controller is mostly identical to SA8775P, but >>> QCS8300 has few additional clocks and minor differences. Hence, reuse >>> SA8775P gpucc bindings and add additional clocks required for QCS8300. >> >> IIUC, these clocks are not valid for SA8775p. How do we deal with such >> cases for other Qualcomm SoCs? >> > > It always possible to add a new platform specific header file and > include the old one. > > For reference see commit a6a61b9701d1 ("dt-bindings: clock: qcom: Add > SM8650 video clock controller"), I believe that's the preferred way > of adding new platform clocks whenever technically possible. > Sure, I will follow the same approach as the commit a6a61b9701d1 and post next series. Thanks, Imran > -- > Best wishes, > Vladimir
This patch series add support for GPUCC, CAMCC and VIDEOCC on Qualcomm QCS8300 platform. Please note that this series is dependent on [1] and [2], which adds support for QCS8300 GCC and SA8775P multi media clock controllers respectively. [1] https://lore.kernel.org/all/20240822-qcs8300-gcc-v2-0-b310dfa70ad8@quicinc.com/ [2] https://lore.kernel.org/all/20241011-sa8775p-mm-v4-resend-patches-v5-0-4a9f17dc683a@quicinc.com/ Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com> --- Changes in v2: - Updated commit text details in bindings patches as per the review comments. - Sorted the compatible order and updated comment in VideoCC driver patch as per the review comments. - Added the R-By tags received in V1. - Link to v1: https://lore.kernel.org/r/20241018-qcs8300-mm-patches-v1-0-859095e0776c@quicinc.com --- Imran Shaik (6): dt-bindings: clock: qcom: Add GPU clocks for QCS8300 clk: qcom: Add support for GPU Clock Controller on QCS8300 dt-bindings: clock: qcom: Add CAMCC clocks for QCS8300 clk: qcom: Add support for Camera Clock Controller on QCS8300 dt-bindings: clock: qcom: Add QCS8300 video clock controller clk: qcom: Add support for Video Clock Controller on QCS8300 .../devicetree/bindings/clock/qcom,gpucc.yaml | 1 + .../bindings/clock/qcom,sa8775p-camcc.yaml | 1 + .../bindings/clock/qcom,sa8775p-videocc.yaml | 1 + drivers/clk/qcom/camcc-sa8775p.c | 99 +++++++++++++++++++++- drivers/clk/qcom/gpucc-sa8775p.c | 47 ++++++++++ drivers/clk/qcom/videocc-sa8775p.c | 8 ++ include/dt-bindings/clock/qcom,sa8775p-camcc.h | 1 + include/dt-bindings/clock/qcom,sa8775p-gpucc.h | 4 +- 8 files changed, 157 insertions(+), 5 deletions(-) --- base-commit: 891a4dc5705df4de9a258accef31786b46700394 change-id: 20241016-qcs8300-mm-patches-fc01e8c75ed4 Best regards,