2022-04-14 11:59:10

by Aswath Govindraju

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Support for polling has been added in the driver, which will be used by
default if interrupts property is not populated. Therefore, remove
interrupts and interrupt-names from the required properties and add a note
under interrupts property describing the above support in driver.

Suggested-by: Roger Quadros <[email protected]>
Signed-off-by: Aswath Govindraju <[email protected]>
---
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index a4c53b1f1af3..1c4b8c6233e5 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -25,6 +25,8 @@ properties:

interrupts:
maxItems: 1
+ description:
+ If interrupts are not populated then by default polling will be used.

interrupt-names:
items:
@@ -33,8 +35,6 @@ properties:
required:
- compatible
- reg
- - interrupts
- - interrupt-names

additionalProperties: true

--
2.17.1


2022-04-15 09:36:45

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi,

On 14/04/2022 11:31, Aswath Govindraju wrote:
> Support for polling has been added in the driver, which will be used by
> default if interrupts property is not populated. Therefore, remove
> interrupts and interrupt-names from the required properties and add a note
> under interrupts property describing the above support in driver.
>
> Suggested-by: Roger Quadros <[email protected]>

I did not suggest to make interrupts optional by default.

What I suggested was that if a DT property exists to explicitly
indicate polling mode then interrupts are not required.

> Signed-off-by: Aswath Govindraju <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> index a4c53b1f1af3..1c4b8c6233e5 100644
> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> @@ -25,6 +25,8 @@ properties:
>
> interrupts:
> maxItems: 1
> + description:
> + If interrupts are not populated then by default polling will be used.
>
> interrupt-names:
> items:
> @@ -33,8 +35,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupts
> - - interrupt-names
>
> additionalProperties: true
>

cheers,
-roger

2022-04-18 20:23:26

by Aswath Govindraju

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi Roger,

On 14/04/22 23:40, Roger Quadros wrote:
> Hi,
>
> On 14/04/2022 11:31, Aswath Govindraju wrote:
>> Support for polling has been added in the driver, which will be used by
>> default if interrupts property is not populated. Therefore, remove
>> interrupts and interrupt-names from the required properties and add a note
>> under interrupts property describing the above support in driver.
>>
>> Suggested-by: Roger Quadros <[email protected]>
>
> I did not suggest to make interrupts optional by default.
>
> What I suggested was that if a DT property exists to explicitly
> indicate polling mode then interrupts are not required.
>

ohh okay, got it. However, may I know if adding a dt property to
indicate polling for aiding the driver, is the correct approach to model it?

In terms of modelling hardware, as interrupts are not connected we are
not populating the interrupts property. Shouldn't that be all. If we are
adding a property explicitly to indicate polling that can be used by
driver, wouldn't that be a software aid being added in the device tree?

Thanks,
Aswath

>> Signed-off-by: Aswath Govindraju <[email protected]>
>> ---
>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>> index a4c53b1f1af3..1c4b8c6233e5 100644
>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>> @@ -25,6 +25,8 @@ properties:
>>
>> interrupts:
>> maxItems: 1
>> + description:
>> + If interrupts are not populated then by default polling will be used.
>>
>> interrupt-names:
>> items:
>> @@ -33,8 +35,6 @@ properties:
>> required:
>> - compatible
>> - reg
>> - - interrupts
>> - - interrupt-names
>>
>> additionalProperties: true
>>
>
> cheers,
> -roger

2022-04-21 01:58:08

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi,

On 18/04/2022 08:19, Aswath Govindraju wrote:
> Hi Roger,
>
> On 14/04/22 23:40, Roger Quadros wrote:
>> Hi,
>>
>> On 14/04/2022 11:31, Aswath Govindraju wrote:
>>> Support for polling has been added in the driver, which will be used by
>>> default if interrupts property is not populated. Therefore, remove
>>> interrupts and interrupt-names from the required properties and add a note
>>> under interrupts property describing the above support in driver.
>>>
>>> Suggested-by: Roger Quadros <[email protected]>
>>
>> I did not suggest to make interrupts optional by default.
>>
>> What I suggested was that if a DT property exists to explicitly
>> indicate polling mode then interrupts are not required.
>>
>
> ohh okay, got it. However, may I know if adding a dt property to
> indicate polling for aiding the driver, is the correct approach to model it?
>
> In terms of modelling hardware, as interrupts are not connected we are
> not populating the interrupts property. Shouldn't that be all. If we are
> adding a property explicitly to indicate polling that can be used by
> driver, wouldn't that be a software aid being added in the device tree?

The hardware (tps6598x chip) has an interrupt pin and is expected to be used
in normal case.

Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
So polling mode is an exception.

cheers,
-roger

>
> Thanks,
> Aswath
>
>>> Signed-off-by: Aswath Govindraju <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>> index a4c53b1f1af3..1c4b8c6233e5 100644
>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>> @@ -25,6 +25,8 @@ properties:
>>>
>>> interrupts:
>>> maxItems: 1
>>> + description:
>>> + If interrupts are not populated then by default polling will be used.
>>>
>>> interrupt-names:
>>> items:
>>> @@ -33,8 +35,6 @@ properties:
>>> required:
>>> - compatible
>>> - reg
>>> - - interrupts
>>> - - interrupt-names
>>>
>>> additionalProperties: true
>>>
>>
>> cheers,
>> -roger

2022-04-22 22:40:25

by Aswath Govindraju

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi Roger,

On 21/04/22 00:46, Roger Quadros wrote:
> Hi,
>
> On 18/04/2022 08:19, Aswath Govindraju wrote:
>> Hi Roger,
>>
>> On 14/04/22 23:40, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 14/04/2022 11:31, Aswath Govindraju wrote:
>>>> Support for polling has been added in the driver, which will be used by
>>>> default if interrupts property is not populated. Therefore, remove
>>>> interrupts and interrupt-names from the required properties and add a note
>>>> under interrupts property describing the above support in driver.
>>>>
>>>> Suggested-by: Roger Quadros <[email protected]>
>>>
>>> I did not suggest to make interrupts optional by default.
>>>
>>> What I suggested was that if a DT property exists to explicitly
>>> indicate polling mode then interrupts are not required.
>>>
>>
>> ohh okay, got it. However, may I know if adding a dt property to
>> indicate polling for aiding the driver, is the correct approach to model it?
>>
>> In terms of modelling hardware, as interrupts are not connected we are
>> not populating the interrupts property. Shouldn't that be all. If we are
>> adding a property explicitly to indicate polling that can be used by
>> driver, wouldn't that be a software aid being added in the device tree?
>
> The hardware (tps6598x chip) has an interrupt pin and is expected to be used
> in normal case.
>
> Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
> So polling mode is an exception.
>

Yes as you mentioned the interrupt line is expected to connected but
there could be cases where there are not enough pins on the SoC and
polling is used intentionally. In these cases this would be a feature
rather than a bug.

Also, I feel like not adding interrupts property in the dt nodes will
indicate polling. My question is why are we adding an extra property
(which is being used only as an aid in the driver) when this feature can
be modeled by making interrupts property optional.

Thanks,
Aswath

> cheers,
> -roger
>
>>
>> Thanks,
>> Aswath
>>
>>>> Signed-off-by: Aswath Govindraju <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>> index a4c53b1f1af3..1c4b8c6233e5 100644
>>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>> @@ -25,6 +25,8 @@ properties:
>>>>
>>>> interrupts:
>>>> maxItems: 1
>>>> + description:
>>>> + If interrupts are not populated then by default polling will be used.
>>>>
>>>> interrupt-names:
>>>> items:
>>>> @@ -33,8 +35,6 @@ properties:
>>>> required:
>>>> - compatible
>>>> - reg
>>>> - - interrupts
>>>> - - interrupt-names
>>>>
>>>> additionalProperties: true
>>>>
>>>
>>> cheers,
>>> -roger


--
Thanks,
Aswath

2022-04-26 13:23:15

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

On Tue, Apr 26, 2022 at 09:42:31AM +0300, Roger Quadros wrote:
> Hi,
>
> On 22/04/2022 08:07, Aswath Govindraju wrote:
> > Hi Roger,
> >
> > On 21/04/22 00:46, Roger Quadros wrote:
> >> Hi,
> >>
> >> On 18/04/2022 08:19, Aswath Govindraju wrote:
> >>> Hi Roger,
> >>>
> >>> On 14/04/22 23:40, Roger Quadros wrote:
> >>>> Hi,
> >>>>
> >>>> On 14/04/2022 11:31, Aswath Govindraju wrote:
> >>>>> Support for polling has been added in the driver, which will be used by
> >>>>> default if interrupts property is not populated. Therefore, remove
> >>>>> interrupts and interrupt-names from the required properties and add a note
> >>>>> under interrupts property describing the above support in driver.
> >>>>>
> >>>>> Suggested-by: Roger Quadros <[email protected]>
> >>>>
> >>>> I did not suggest to make interrupts optional by default.
> >>>>
> >>>> What I suggested was that if a DT property exists to explicitly
> >>>> indicate polling mode then interrupts are not required.
> >>>>
> >>>
> >>> ohh okay, got it. However, may I know if adding a dt property to
> >>> indicate polling for aiding the driver, is the correct approach to model it?
> >>>
> >>> In terms of modelling hardware, as interrupts are not connected we are
> >>> not populating the interrupts property. Shouldn't that be all. If we are
> >>> adding a property explicitly to indicate polling that can be used by
> >>> driver, wouldn't that be a software aid being added in the device tree?
> >>
> >> The hardware (tps6598x chip) has an interrupt pin and is expected to be used
> >> in normal case.
> >>
> >> Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
> >> So polling mode is an exception.
> >>
> >
> > Yes as you mentioned the interrupt line is expected to connected but
> > there could be cases where there are not enough pins on the SoC and
> > polling is used intentionally. In these cases this would be a feature
> > rather than a bug.
>
> I do not agree that this is a feature but a board defect. You can always use
> a GPIO expander to add more GPIOs than the SoC can provide.
>
> Type-C events are asynchronous and polling is a waste of CPU time.
> What will you do if system suspends and you need to wake up on Type-C
> status change?
> So polling mode is just an exception for the defective boards or could
> be used for debugging.
>
> >
> > Also, I feel like not adding interrupts property in the dt nodes will
> > indicate polling. My question is why are we adding an extra property
> > (which is being used only as an aid in the driver) when this feature can
> > be modeled by making interrupts property optional.
>
> Because interrupt property was not originally optional for this driver.
>
> I would like to hear what Heikki has to say about this.
>
> Any thoughts Heikki?

I think the question is generic. How should DT describe the
connection/lack of connection? Rob should comment on this.

thanks,


> cheers,
> -roger
>
> >
> > Thanks,
> > Aswath
> >
> >> cheers,
> >> -roger
> >>
> >>>
> >>> Thanks,
> >>> Aswath
> >>>
> >>>>> Signed-off-by: Aswath Govindraju <[email protected]>
> >>>>> ---
> >>>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
> >>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> >>>>> index a4c53b1f1af3..1c4b8c6233e5 100644
> >>>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> >>>>> @@ -25,6 +25,8 @@ properties:
> >>>>>
> >>>>> interrupts:
> >>>>> maxItems: 1
> >>>>> + description:
> >>>>> + If interrupts are not populated then by default polling will be used.
> >>>>>
> >>>>> interrupt-names:
> >>>>> items:
> >>>>> @@ -33,8 +35,6 @@ properties:
> >>>>> required:
> >>>>> - compatible
> >>>>> - reg
> >>>>> - - interrupts
> >>>>> - - interrupt-names
> >>>>>
> >>>>> additionalProperties: true
> >>>>>
> >>>>
> >>>> cheers,
> >>>> -roger
> >
> >

--
heikki

2022-04-26 13:49:18

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi,

On 22/04/2022 08:07, Aswath Govindraju wrote:
> Hi Roger,
>
> On 21/04/22 00:46, Roger Quadros wrote:
>> Hi,
>>
>> On 18/04/2022 08:19, Aswath Govindraju wrote:
>>> Hi Roger,
>>>
>>> On 14/04/22 23:40, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 14/04/2022 11:31, Aswath Govindraju wrote:
>>>>> Support for polling has been added in the driver, which will be used by
>>>>> default if interrupts property is not populated. Therefore, remove
>>>>> interrupts and interrupt-names from the required properties and add a note
>>>>> under interrupts property describing the above support in driver.
>>>>>
>>>>> Suggested-by: Roger Quadros <[email protected]>
>>>>
>>>> I did not suggest to make interrupts optional by default.
>>>>
>>>> What I suggested was that if a DT property exists to explicitly
>>>> indicate polling mode then interrupts are not required.
>>>>
>>>
>>> ohh okay, got it. However, may I know if adding a dt property to
>>> indicate polling for aiding the driver, is the correct approach to model it?
>>>
>>> In terms of modelling hardware, as interrupts are not connected we are
>>> not populating the interrupts property. Shouldn't that be all. If we are
>>> adding a property explicitly to indicate polling that can be used by
>>> driver, wouldn't that be a software aid being added in the device tree?
>>
>> The hardware (tps6598x chip) has an interrupt pin and is expected to be used
>> in normal case.
>>
>> Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
>> So polling mode is an exception.
>>
>
> Yes as you mentioned the interrupt line is expected to connected but
> there could be cases where there are not enough pins on the SoC and
> polling is used intentionally. In these cases this would be a feature
> rather than a bug.

I do not agree that this is a feature but a board defect. You can always use
a GPIO expander to add more GPIOs than the SoC can provide.

Type-C events are asynchronous and polling is a waste of CPU time.
What will you do if system suspends and you need to wake up on Type-C
status change?
So polling mode is just an exception for the defective boards or could
be used for debugging.

>
> Also, I feel like not adding interrupts property in the dt nodes will
> indicate polling. My question is why are we adding an extra property
> (which is being used only as an aid in the driver) when this feature can
> be modeled by making interrupts property optional.

Because interrupt property was not originally optional for this driver.

I would like to hear what Heikki has to say about this.

Any thoughts Heikki?

cheers,
-roger

>
> Thanks,
> Aswath
>
>> cheers,
>> -roger
>>
>>>
>>> Thanks,
>>> Aswath
>>>
>>>>> Signed-off-by: Aswath Govindraju <[email protected]>
>>>>> ---
>>>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>> index a4c53b1f1af3..1c4b8c6233e5 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>> @@ -25,6 +25,8 @@ properties:
>>>>>
>>>>> interrupts:
>>>>> maxItems: 1
>>>>> + description:
>>>>> + If interrupts are not populated then by default polling will be used.
>>>>>
>>>>> interrupt-names:
>>>>> items:
>>>>> @@ -33,8 +35,6 @@ properties:
>>>>> required:
>>>>> - compatible
>>>>> - reg
>>>>> - - interrupts
>>>>> - - interrupt-names
>>>>>
>>>>> additionalProperties: true
>>>>>
>>>>
>>>> cheers,
>>>> -roger
>
>

2022-05-02 23:09:32

by Aswath Govindraju

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi Rob,

On 26/04/22 12:27, Heikki Krogerus wrote:
> On Tue, Apr 26, 2022 at 09:42:31AM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 22/04/2022 08:07, Aswath Govindraju wrote:
>>> Hi Roger,
>>>
>>> On 21/04/22 00:46, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 18/04/2022 08:19, Aswath Govindraju wrote:
>>>>> Hi Roger,
>>>>>
>>>>> On 14/04/22 23:40, Roger Quadros wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 14/04/2022 11:31, Aswath Govindraju wrote:
>>>>>>> Support for polling has been added in the driver, which will be used by
>>>>>>> default if interrupts property is not populated. Therefore, remove
>>>>>>> interrupts and interrupt-names from the required properties and add a note
>>>>>>> under interrupts property describing the above support in driver.
>>>>>>>
>>>>>>> Suggested-by: Roger Quadros <[email protected]>
>>>>>>
>>>>>> I did not suggest to make interrupts optional by default.
>>>>>>
>>>>>> What I suggested was that if a DT property exists to explicitly
>>>>>> indicate polling mode then interrupts are not required.
>>>>>>
>>>>>
>>>>> ohh okay, got it. However, may I know if adding a dt property to
>>>>> indicate polling for aiding the driver, is the correct approach to model it?
>>>>>
>>>>> In terms of modelling hardware, as interrupts are not connected we are
>>>>> not populating the interrupts property. Shouldn't that be all. If we are
>>>>> adding a property explicitly to indicate polling that can be used by
>>>>> driver, wouldn't that be a software aid being added in the device tree?
>>>>
>>>> The hardware (tps6598x chip) has an interrupt pin and is expected to be used
>>>> in normal case.
>>>>
>>>> Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
>>>> So polling mode is an exception.
>>>>
>>>
>>> Yes as you mentioned the interrupt line is expected to connected but
>>> there could be cases where there are not enough pins on the SoC and
>>> polling is used intentionally. In these cases this would be a feature
>>> rather than a bug.
>>
>> I do not agree that this is a feature but a board defect. You can always use
>> a GPIO expander to add more GPIOs than the SoC can provide.
>>
>> Type-C events are asynchronous and polling is a waste of CPU time.
>> What will you do if system suspends and you need to wake up on Type-C
>> status change?
>> So polling mode is just an exception for the defective boards or could
>> be used for debugging.
>>
>>>
>>> Also, I feel like not adding interrupts property in the dt nodes will
>>> indicate polling. My question is why are we adding an extra property
>>> (which is being used only as an aid in the driver) when this feature can
>>> be modeled by making interrupts property optional.
>>
>> Because interrupt property was not originally optional for this driver.
>>
>> I would like to hear what Heikki has to say about this.
>>
>> Any thoughts Heikki?
>
> I think the question is generic. How should DT describe the
> connection/lack of connection? Rob should comment on this.
>

A gentle ping regarding this.

Thanks,
Aswath

> thanks,
>
>
>> cheers,
>> -roger
>>
>>>
>>> Thanks,
>>> Aswath
>>>
>>>> cheers,
>>>> -roger
>>>>
>>>>>
>>>>> Thanks,
>>>>> Aswath
>>>>>
>>>>>>> Signed-off-by: Aswath Govindraju <[email protected]>
>>>>>>> ---
>>>>>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>> index a4c53b1f1af3..1c4b8c6233e5 100644
>>>>>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>> @@ -25,6 +25,8 @@ properties:
>>>>>>>
>>>>>>> interrupts:
>>>>>>> maxItems: 1
>>>>>>> + description:
>>>>>>> + If interrupts are not populated then by default polling will be used.
>>>>>>>
>>>>>>> interrupt-names:
>>>>>>> items:
>>>>>>> @@ -33,8 +35,6 @@ properties:
>>>>>>> required:
>>>>>>> - compatible
>>>>>>> - reg
>>>>>>> - - interrupts
>>>>>>> - - interrupt-names
>>>>>>>
>>>>>>> additionalProperties: true
>>>>>>>
>>>>>>
>>>>>> cheers,
>>>>>> -roger
>>>
>>>
>

2022-06-09 06:19:24

by Aswath Govindraju

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: usb: tps6598x: Make the interrupts property optional

Hi Rob,

On 02/05/22 11:00, Aswath Govindraju wrote:
> Hi Rob,
>
> On 26/04/22 12:27, Heikki Krogerus wrote:
>> On Tue, Apr 26, 2022 at 09:42:31AM +0300, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 22/04/2022 08:07, Aswath Govindraju wrote:
>>>> Hi Roger,
>>>>
>>>> On 21/04/22 00:46, Roger Quadros wrote:
>>>>> Hi,
>>>>>
>>>>> On 18/04/2022 08:19, Aswath Govindraju wrote:
>>>>>> Hi Roger,
>>>>>>
>>>>>> On 14/04/22 23:40, Roger Quadros wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 14/04/2022 11:31, Aswath Govindraju wrote:
>>>>>>>> Support for polling has been added in the driver, which will be used by
>>>>>>>> default if interrupts property is not populated. Therefore, remove
>>>>>>>> interrupts and interrupt-names from the required properties and add a note
>>>>>>>> under interrupts property describing the above support in driver.
>>>>>>>>
>>>>>>>> Suggested-by: Roger Quadros <[email protected]>
>>>>>>>
>>>>>>> I did not suggest to make interrupts optional by default.
>>>>>>>
>>>>>>> What I suggested was that if a DT property exists to explicitly
>>>>>>> indicate polling mode then interrupts are not required.
>>>>>>>
>>>>>>
>>>>>> ohh okay, got it. However, may I know if adding a dt property to
>>>>>> indicate polling for aiding the driver, is the correct approach to model it?
>>>>>>
>>>>>> In terms of modelling hardware, as interrupts are not connected we are
>>>>>> not populating the interrupts property. Shouldn't that be all. If we are
>>>>>> adding a property explicitly to indicate polling that can be used by
>>>>>> driver, wouldn't that be a software aid being added in the device tree?
>>>>>
>>>>> The hardware (tps6598x chip) has an interrupt pin and is expected to be used
>>>>> in normal case.
>>>>>
>>>>> Some buggy boards might have forgot to connect it. We are adding polling mode only for these buggy boards. ;)
>>>>> So polling mode is an exception.
>>>>>
>>>>
>>>> Yes as you mentioned the interrupt line is expected to connected but
>>>> there could be cases where there are not enough pins on the SoC and
>>>> polling is used intentionally. In these cases this would be a feature
>>>> rather than a bug.
>>>
>>> I do not agree that this is a feature but a board defect. You can always use
>>> a GPIO expander to add more GPIOs than the SoC can provide.
>>>
>>> Type-C events are asynchronous and polling is a waste of CPU time.
>>> What will you do if system suspends and you need to wake up on Type-C
>>> status change?
>>> So polling mode is just an exception for the defective boards or could
>>> be used for debugging.
>>>
>>>>
>>>> Also, I feel like not adding interrupts property in the dt nodes will
>>>> indicate polling. My question is why are we adding an extra property
>>>> (which is being used only as an aid in the driver) when this feature can
>>>> be modeled by making interrupts property optional.
>>>
>>> Because interrupt property was not originally optional for this driver.
>>>
>>> I would like to hear what Heikki has to say about this.
>>>
>>> Any thoughts Heikki?
>>
>> I think the question is generic. How should DT describe the
>> connection/lack of connection? Rob should comment on this.
>>
>
> A gentle ping regarding this.
>

A gentle ping regarding the above question.

Thanks,
Aswath

> Thanks,
> Aswath
>
>> thanks,
>>
>>
>>> cheers,
>>> -roger
>>>
>>>>
>>>> Thanks,
>>>> Aswath
>>>>
>>>>> cheers,
>>>>> -roger
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Aswath
>>>>>>
>>>>>>>> Signed-off-by: Aswath Govindraju <[email protected]>
>>>>>>>> ---
>>>>>>>> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 4 ++--
>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>>> index a4c53b1f1af3..1c4b8c6233e5 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
>>>>>>>> @@ -25,6 +25,8 @@ properties:
>>>>>>>>
>>>>>>>> interrupts:
>>>>>>>> maxItems: 1
>>>>>>>> + description:
>>>>>>>> + If interrupts are not populated then by default polling will be used.
>>>>>>>>
>>>>>>>> interrupt-names:
>>>>>>>> items:
>>>>>>>> @@ -33,8 +35,6 @@ properties:
>>>>>>>> required:
>>>>>>>> - compatible
>>>>>>>> - reg
>>>>>>>> - - interrupts
>>>>>>>> - - interrupt-names
>>>>>>>>
>>>>>>>> additionalProperties: true
>>>>>>>>
>>>>>>>
>>>>>>> cheers,
>>>>>>> -roger
>>>>
>>>>
>>


--
Thanks,
Aswath