[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cc73a82d-89fa-1edf-650a-a1a3824cc791@sberdevices.ru>
Date: Wed, 8 Mar 2023 12:16:00 +0000
From: George Stark <GNStark@...rdevices.ru>
To: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
CC: "thierry.reding@...il.com" <thierry.reding@...il.com>,
"krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-samsung-soc@...r.kernel.org"
<linux-samsung-soc@...r.kernel.org>, kernel <kernel@...rdevices.ru>
Subject: Re: [RFC PATCH v1] Revert "pwm: Clear chip_data in pwm_put()"
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@...rdevices.ru>
>>
>> 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
Powered by blists - more mailing lists