On 18/06/2024 00:33, Dmitry Baryshkov wrote: > On Mon, Jun 17, 2024 at 06:03:02PM GMT, Marc Gonzalez wrote: > >> + if (sbridge->vcc) { >> + ret = regulator_enable(sbridge->vcc); >> + msleep(100); > > At least this should be documented or explained in the commit message. > Is it absolutely necessary? Can you use regulator-enable-ramp-delay or > any other DT property instead? The value comes from datasheet "8.3.2 Operation Timing" Table 1. Power Up and Operation Timing Requirements VDD supply ramp up requirements, max = 100 ms VCC supply ramp up requirements, max = 100 ms Did I read the spec wrong? (Very possible) Are you saying this could/should be a property of the regulator? What if the regulator gates several different blocks? >> sbridge = devm_kzalloc(dev, sizeof(*sbridge), GFP_KERNEL); >> if (!sbridge) >> return -ENOMEM; >> - platform_set_drvdata(pdev, sbridge); > > I think this call can get dropped together with the remove() being > gone... Oooh, it didn't occur to me that the only reason to store drvdata was to have it available in the remove callback... > Does this work if the driver is built as a module? Not sure there's any point in testing since Maxime NACKed the approach. Regards