ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> пише: > > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote: > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko > > <andriy.shevchenko@xxxxxxxxxxxxxxx> пише: > > > On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote: > > > > Remove platform data and fully relay on OF and device tree > > > > parsing and binding devices. > > > > > > Thanks for following the advice, but the problem with this change as it does > > > too much at once. It should be split to a few simpler ones. > > > On top of that, this removes MFD participation from the driver but leaves it > > > under MFD realm. Moreover, looking briefly at the code it looks like it open > > > codes the parts of MFD. The latter needs a very goo justification which commit > > > message is missing. > > ... > > > Splitting this into a set of commits would be nearly impossible, > > I don't buy this. > One patch can introduce device property support. > Another one removes the old platform data interface. > > So, at bare minimum there will be two patches. (Besides the advice to have > two more.) > > > original driver does not relay on OF, it relays on platform data. > > And?.. > > > Ripping out platform data will leave behind a broken useless driver. > > Hmm... This cna be the case if and only if we have the user in kernel. > Is this the case? > > > So it has to be done simultaneously. > > Nope. > > > MFD part is removed since MFD cells binding is unconditional, while > > the device supports any amount of children grater then one. For > > example, my device uses only backlight at bank A, while all other > > subdevices are not present and used. This patch switches to dynamic > > bind of children. > > MFD does the same. Please, take your time and get familiar with how MFD works. > It does not. I have tried. If mfd cell binding is missing, driver will complain and fail. > ... > > > > > + device_property_read_string(&pdev->dev, "linux,default-trigger", > > > > + &led->cdev.default_trigger); > > > > > > One prerequisite patch you probably want is an introduction of > > > > > > struct device *dev = &pdev->dev; > > > > > > in the respective ->probe() implementations. This, in particular, makes the > > > above lines shorter and fit one line. > > > > This is not a scope of this patchset. Original driver uses &pdev->dev > > Indirectly it is. The change you are proposing tries to continue using this > construction with making needlessly longer. > > ... > > > > > + if (!strcmp(comatible, "ti,lm3533-als")) > > > > + lm3533->have_als = 1; > > > > > > If you end up having this, it's not the best what we can do. OF ID tables have > > > a driver_data field exactly for the cases like this. > > > > This is required by core driver to handle some attributes and is here > > solely not to touch those in this patch. > > What does this mean? > > -- > With Best Regards, > Andy Shevchenko > > Let this driver rot for now, I might return to it. At some point
