Re: [RFC PATCH v1] Revert "pwm: Clear chip_data in pwm_put()"
From: George Stark
Date: Wed Mar 08 2023 - 07:16:15 EST
Hello Uwe
On 3/8/23 00:28, Uwe Kleine-König wrote:
> Hello George,
>
> On Wed, Mar 08, 2023 at 12:00:14AM +0300, George Stark wrote:
>> From: George Stark <GNStark@xxxxxxxxxxxxxx>
>>
>> This reverts commit e926b12c611c2095c7976e2ed31753ad6eb5ff1a.
>>
>> There're several issues with the original change:
>>
>> - it breaks generic semantics of set_driver_data-like routines that
>> only client code controls lifetime of it's own data.
>>
>> - it breaks pwm-sti.c driver: pwm_set_chip_data is used only in probe stage
>> then pwm_get_chip_data used in capture callback
> pwm-sti is also broken in other regards. pwm_set_chip_data() is only
> called after pwmchip_add(). But as soon as pwmchip_add() is called, the
> callbacks (e.g. capture) might be called. Then the call to
> pwm_set_chip_data() might be too late.
>
>> Change-Id: I5787c6b4c520d4a0997567c416b26fa4e0806b94
> Please don't add a Change-Id footer for Linux submissions.
missed it somehow. Sorry about that
>
> If you ask me, better drop pwm_set_chip_data() completely. It adds no
> useful value. It's just a variant of driver data and using both
> complicates the driver and probably fragments memory allocations. Also
> the sematic of driver data is better known as it's the same for all
> subsystems.
>
> Do you use the capture functionality? In my eyes the capture part of the
> pwm subsystem is very alien. Only a small subset of the hardware
> supports this and the counter framework should be better suited for such
> tasks.
I don't use pwm-sti driver. I update meson pwm driver for new chips
and when started using pwm_set_chip_data in probe I was very surprised that
my data is lost after sysfs export/unexport calls. Then I found the
patch and
checked other drivers for similar usecases.
Probably you're right about dropping pwm_set_chip_data.
> Best regards
> Uwe
>
Best regards
George