[PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'

Frank Li posted 3 patches 2 months, 2 weeks ago
[PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Frank Li 2 months, 2 weeks ago
The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
increase maxItems for clock and clock-names.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v1 to v2
- remove compatible string fsl,imx8qxp-mipi-pwm
---
 Documentation/devicetree/bindings/pwm/imx-pwm.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml
index 04148198e34d0..bc6991bd466e1 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml
@@ -51,11 +51,15 @@ properties:
     items:
       - description: SoC PWM ipg clock
       - description: SoC PWM per clock
+      - description: 32k clock
+    minItems: 2
 
   clock-names:
     items:
       - const: ipg
       - const: per
+      - const: 32k
+    minItems: 2
 
   interrupts:
     maxItems: 1

-- 
2.34.1
Re: [PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Marek Vasut 2 months, 2 weeks ago
On 9/10/24 9:07 PM, Frank Li wrote:
> The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
> increase maxItems for clock and clock-names.

This mentions MIPI subsystem, but the IP in question here is PWM.

Are you sure the clock are assigned to the correct IP ?

Shouldn't the clock be assigned to some MIPI IP instead ?

Could you please clarify this in the commit message ?
Re: [PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Frank Li 2 months, 2 weeks ago
On Wed, Sep 11, 2024 at 10:28:52PM +0200, Marek Vasut wrote:
> On 9/10/24 9:07 PM, Frank Li wrote:
> > The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
> > increase maxItems for clock and clock-names.
>
> This mentions MIPI subsystem, but the IP in question here is PWM.

Here, mipi just name of subsystem, not related MIPI IP at all.

There are many IP in i.MX8QXP mipi subsystem, such as i2c, PWM, MIPI PHY,
MIPI controller, PLL, clock-gate, ...

>
> Are you sure the clock are assigned to the correct IP ?
>
> Shouldn't the clock be assigned to some MIPI IP instead ?
>

Are both question still validate if treat 'mipi' just name of subsystem.

> Could you please clarify this in the commit message ?

'mipi' just name of subsystem because the major ip is for MIPI. is word
'mipi-subsystem' better?


Frank
Re: [PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Marek Vasut 2 months, 2 weeks ago
On 9/11/24 11:10 PM, Frank Li wrote:
> On Wed, Sep 11, 2024 at 10:28:52PM +0200, Marek Vasut wrote:
>> On 9/10/24 9:07 PM, Frank Li wrote:
>>> The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
>>> increase maxItems for clock and clock-names.
>>
>> This mentions MIPI subsystem, but the IP in question here is PWM.
> 
> Here, mipi just name of subsystem, not related MIPI IP at all.
> 
> There are many IP in i.MX8QXP mipi subsystem, such as i2c, PWM, MIPI PHY,
> MIPI controller, PLL, clock-gate, ...
> 
>>
>> Are you sure the clock are assigned to the correct IP ?
>>
>> Shouldn't the clock be assigned to some MIPI IP instead ?
>>
> 
> Are both question still validate if treat 'mipi' just name of subsystem.
> 
>> Could you please clarify this in the commit message ?
> 
> 'mipi' just name of subsystem because the major ip is for MIPI. is word
> 'mipi-subsystem' better?
Let's find out.

What is the 32kHz clock used for in the PWM block ?
Re: [PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Frank Li 2 months, 2 weeks ago
On Wed, Sep 11, 2024 at 11:14:15PM +0200, Marek Vasut wrote:
> On 9/11/24 11:10 PM, Frank Li wrote:
> > On Wed, Sep 11, 2024 at 10:28:52PM +0200, Marek Vasut wrote:
> > > On 9/10/24 9:07 PM, Frank Li wrote:
> > > > The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
> > > > increase maxItems for clock and clock-names.
> > >
> > > This mentions MIPI subsystem, but the IP in question here is PWM.
> >
> > Here, mipi just name of subsystem, not related MIPI IP at all.
> >
> > There are many IP in i.MX8QXP mipi subsystem, such as i2c, PWM, MIPI PHY,
> > MIPI controller, PLL, clock-gate, ...
> >
> > >
> > > Are you sure the clock are assigned to the correct IP ?
> > >
> > > Shouldn't the clock be assigned to some MIPI IP instead ?
> > >
> >
> > Are both question still validate if treat 'mipi' just name of subsystem.
> >
> > > Could you please clarify this in the commit message ?
> >
> > 'mipi' just name of subsystem because the major ip is for MIPI. is word
> > 'mipi-subsystem' better?
> Let's find out.
>
> What is the 32kHz clock used for in the PWM block ?

After read document again, it is one option of input, CLKSRC in PWMCR.

Frank
Re: [PATCH v3 1/3] dt-bindings: pwm: imx: Add optional clock '32k'
Posted by Marek Vasut 2 months, 2 weeks ago
On 9/12/24 12:19 AM, Frank Li wrote:
> On Wed, Sep 11, 2024 at 11:14:15PM +0200, Marek Vasut wrote:
>> On 9/11/24 11:10 PM, Frank Li wrote:
>>> On Wed, Sep 11, 2024 at 10:28:52PM +0200, Marek Vasut wrote:
>>>> On 9/10/24 9:07 PM, Frank Li wrote:
>>>>> The pwm in imx8qxp mipi subsystem require one extra '32k' clock. So
>>>>> increase maxItems for clock and clock-names.
>>>>
>>>> This mentions MIPI subsystem, but the IP in question here is PWM.
>>>
>>> Here, mipi just name of subsystem, not related MIPI IP at all.
>>>
>>> There are many IP in i.MX8QXP mipi subsystem, such as i2c, PWM, MIPI PHY,
>>> MIPI controller, PLL, clock-gate, ...
>>>
>>>>
>>>> Are you sure the clock are assigned to the correct IP ?
>>>>
>>>> Shouldn't the clock be assigned to some MIPI IP instead ?
>>>>
>>>
>>> Are both question still validate if treat 'mipi' just name of subsystem.
>>>
>>>> Could you please clarify this in the commit message ?
>>>
>>> 'mipi' just name of subsystem because the major ip is for MIPI. is word
>>> 'mipi-subsystem' better?
>> Let's find out.
>>
>> What is the 32kHz clock used for in the PWM block ?
> 
> After read document again, it is one option of input, CLKSRC in PWMCR.
Thank you for checking.

It seems PWMCR CLKSRC is currently hard-coded to IPG_HIGH in this PWM 
driver, so the 32kHz clock are currently not used ?

The question is, does it make sense to add them ? And if so, what would 
be the use case compared to current IPG_HIGH ?