Hi Laurent On 25/08/2021 14:59, Laurent Pinchart wrote: > Hello, > > CC'ing Sakari. > > On Wed, Aug 25, 2021 at 02:11:39PM +0100, Mark Brown wrote: >> On Wed, Aug 25, 2021 at 03:26:37PM +0300, Andy Shevchenko wrote: >>> On Wed, Aug 25, 2021 at 2:30 PM Mark Brown <broonie@xxxxxxxxxx> wrote: >>>> No, what was proposed for regulator was to duplicate all the the DT >>>> binding code in the regulator framework so it parses fwnodes then have >>>> an API for encoding fwnodes from C data structures at runtime. The bit >>>> where the data gets joined up with the devices isn't the problem, it's >>>> the duplication and fragility introduced by encoding everything into >>>> an intermediate representation that has no purpose and passing that >>>> around which is the problem. >>> The whole exercise with swnode is to minimize the driver intrusion and >>> evolving a unified way for (some) of the device properties. V4L2 won't >> The practical implementation for regulators was to duplicate a >> substantial amount of code in the core in order to give us a less type >> safe and more indirect way of passing data from onen C file in the >> kernel to another. This proposal is a lot better in that it uses the >> existing init_data and avoids the huge amounts of duplication, it's just >> not clear from the changelog why it's doing this in a regulator specific >> manner. >> >> *Please* stop trying to force swnodes in everywhere, take on board the >> feedback about why the swnode implementation is completely inappropriate >> for regulators. I don't understand why you continue to push this so >> hard. swnodes and fwnodes are a solution to a specific problem, they're >> not the answer to every problem out there and having to rehash this >> continually is getting in the way of actually discussing practical >> workarounds for these poorly implemented ACPI platforms. >> >>> like what you are suggesting exactly because they don't like the idea >>> of spreading the board code over the drivers. In some cases it might >>> even be not so straightforward and easy. >>> Laurent, do I understand correctly the v4l2 expectations? >> There will be some cases where swnodes make sense, for example where the >> data is going to be read through the fwnode API since the binding is >> firmware neutral which I think is the v4l case. On the other hand >> having a direct C representation is a very common way of implementing >> DMI quirk tables, and we have things like the regulator API where >> there's off the shelf platform data support and we actively don't want >> to support fwnode. > From a camera sensor point of view, we want to avoid code duplication. > Having to look for regulators using OF lookups *and* platform data in > every single sensor driver is not a good solution. This means that, from > a camera sensor driver point of view, we want to call regulator_get() > (or the devm_ version) with a name, without caring about who establishes > the mapping and how the lookup is performed. I don't care much > personally if this would be implemented through swnode or a different > mechanism, as long as the implementation can be centralized. I think rather than sensor drivers, the idea would be just to have the tps68470-regulator driver check platform data for the init_data instead, like the tps65023-regulator driver [1] does. I'm sure that'll work, but it's not particularly centralized from the regulator driver's point of view. [1] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/tps65023-regulator.c#L268
