| Message ID | 20210329105047.51033-1-heikki.krogerus@linux.intel.com |
|---|---|
| Headers | show |
| Series | i2c: Adding support for software nodes | expand |
On 29/03/2021 12:50, Heikki Krogerus wrote: > Additional device properties are always just a part of a > software fwnode. If the device properties are constant, the > software node can also be constant. > Hi, Thanks for your work. I did not get the cover letter nor other patches from this set and I don't see how the i2c uses the swnode. This makes difficult to judge whether this looks reasonable. At least without the context the title looks misleading - you add software_node or change to use software_node instead of constifying it. Best regards, Krzysztof
On Mon, Mar 29, 2021 at 12:58:41PM +0200, Krzysztof Kozlowski wrote: > On 29/03/2021 12:50, Heikki Krogerus wrote: > > Additional device properties are always just a part of a > > software fwnode. If the device properties are constant, the > > software node can also be constant. > > > Hi, > > Thanks for your work. > > I did not get the cover letter nor other patches from this set and I > don't see how the i2c uses the swnode. This makes difficult to judge > whether this looks reasonable. At least without the context the title > looks misleading - you add software_node or change to use software_node > instead of constifying it. OK, I'll try to open this up somehow... Whenever additional device properties are added to devices by using the old device property API (device_add_properties()) that also i2c core code uses, in reality a software node is always created to hold those properties. It's just always dynamically allocated. The goal of this series is to prepare the i2c subsystem and drivers for the removal of that old device property API, but I did not see that as relevant info for this patch, because even if we did not in the end remove that old API, this change is still useful. The patch does exactly what the subject says. After this we supply the device a constant software node instead of a dynamically allocated one. thanks,
Hi, On 3/29/21 12:50 PM, Heikki Krogerus wrote: > Additional device properties are always just a part of a > software fwnode. If the device properties are constant, the > software node can also be constant. > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Hans de Goede <hdegoede@redhat.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/i2c/busses/i2c-cht-wc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c > index f80d79e973cd2..08f491ea21ac9 100644 > --- a/drivers/i2c/busses/i2c-cht-wc.c > +++ b/drivers/i2c/busses/i2c-cht-wc.c > @@ -280,6 +280,10 @@ static const struct property_entry bq24190_props[] = { > { } > }; > > +static const struct software_node bq24190_node = { > + .properties = bq24190_props, > +}; > + > static struct regulator_consumer_supply fusb302_consumer = { > .supply = "vbus", > /* Must match fusb302 dev_name in intel_cht_int33fe.c */ > @@ -308,7 +312,7 @@ static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev) > .type = "bq24190", > .addr = 0x6b, > .dev_name = "bq24190", > - .properties = bq24190_props, > + .swnode = &bq24190_node, > .platform_data = &bq24190_pdata, > }; > int ret, reg, irq; >
This looks great, thank you for constifying this. Finally it's clean again, yet much more flexible than the original platform_data approach when I first submitted the driver. Thanks everyone for your work Max
> The old device property API (device_add_properties()) is going to be > removed. These prepare the i2c subsystem and drivers for the change. > The change is fairly trivial in case of i2c. All we need to do is add > complete software nodes to the devices instead of only the device > properties in those nodes. This looks like a nice cleanup! Reviewed-by: Wolfram Sang <wsa@kernel.org> # for the I2C parts Which tree should this go into? I can offer I2C but am also fine with another one...
On Wed, Mar 31, 2021 at 11:22:32AM +0200, Wolfram Sang wrote: > > > The old device property API (device_add_properties()) is going to be > > removed. These prepare the i2c subsystem and drivers for the change. > > The change is fairly trivial in case of i2c. All we need to do is add > > complete software nodes to the devices instead of only the device > > properties in those nodes. > > This looks like a nice cleanup! > > Reviewed-by: Wolfram Sang <wsa@kernel.org> # for the I2C parts > > Which tree should this go into? I can offer I2C but am also fine with > another one... I think these go via I2C tree. thanks, -- heikki
On 29/03/2021 14:32, Heikki Krogerus wrote: > On Mon, Mar 29, 2021 at 12:58:41PM +0200, Krzysztof Kozlowski wrote: >> On 29/03/2021 12:50, Heikki Krogerus wrote: >>> Additional device properties are always just a part of a >>> software fwnode. If the device properties are constant, the >>> software node can also be constant. >>> >> Hi, >> >> Thanks for your work. >> >> I did not get the cover letter nor other patches from this set and I >> don't see how the i2c uses the swnode. This makes difficult to judge >> whether this looks reasonable. At least without the context the title >> looks misleading - you add software_node or change to use software_node >> instead of constifying it. > > OK, I'll try to open this up somehow... > > Whenever additional device properties are added to devices by using > the old device property API (device_add_properties()) that also i2c > core code uses, in reality a software node is always created to hold > those properties. It's just always dynamically allocated. > > The goal of this series is to prepare the i2c subsystem and drivers > for the removal of that old device property API, but I did not see > that as relevant info for this patch, because even if we did not in > the end remove that old API, this change is still useful. > > The patch does exactly what the subject says. After this we supply the > device a constant software node instead of a dynamically allocated one. Thanks for explanation. The follow up question is - can I take it independently via ARM Samsung/S3C tree? Best regards, Krzysztof
> I think these go via I2C tree.
Good, I'll apply it this weekend. Until then, let's hope we can get some
more acks.
> Thanks for explanation. The follow up question is - can I take it > independently via ARM Samsung/S3C tree? Is it possible to just ack it, so I can take this all via I2C? Or will there be merge conflicts. I can provide an immutable branch, of course.
On 06/04/2021 21:40, Wolfram Sang wrote: > >> Thanks for explanation. The follow up question is - can I take it >> independently via ARM Samsung/S3C tree? > > Is it possible to just ack it, so I can take this all via I2C? Or will > there be merge conflicts. I can provide an immutable branch, of course. > Sure, ack is possible (from my currently used email): Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
On Tue, Mar 30, 2021 at 03:58:56PM +0200, Max Staudt wrote: > This looks great, thank you for constifying this. > > Finally it's clean again, yet much more flexible than the original > platform_data approach when I first submitted the driver. I read this as "Reviewed-by" ;)
On Mon, Mar 29, 2021 at 01:50:35PM +0300, Heikki Krogerus wrote: > Hi, > > The old device property API (device_add_properties()) is going to be > removed. These prepare the i2c subsystem and drivers for the change. > The change is fairly trivial in case of i2c. All we need to do is add > complete software nodes to the devices instead of only the device > properties in those nodes. > > thanks, > > Heikki Krogerus (12): > i2c: Add support for software nodes > ARM: davinci: Constify the software nodes > ARM: omap1: osk: Constify the software node > ARM: pxa: stargate2: Constify the software node > ARM: s3c: mini2440: Constify the software node > platform/x86: intel_cht_int33fe_microb: Constify the software node > i2c: cht-wc: Constify the software node > i2c: nvidia-gpu: Constify the software node > i2c: icy: Constify the software node > platform/chrome: chromeos_laptop - Prepare complete software nodes > Input: elantech - Prepare a complete software node for the device > i2c: Remove support for dangling device properties I applied all patches to an immutable branch "i2c/software-nodes" now. Once buildbot successfully checked this branch, I will merge it into for-next and advertise it to interested parties. Thanks for this work!
On Thu, 8 Apr 2021 23:22:51 +0200 Wolfram Sang <wsa@kernel.org> wrote: > I read this as "Reviewed-by" ;) Sure, why not :) Reviewed-by: Max Staudt <max@enpas.org>
On Thu, Apr 08, 2021 at 11:53:23PM +0200, Wolfram Sang wrote: > On Mon, Mar 29, 2021 at 01:50:35PM +0300, Heikki Krogerus wrote: > > Hi, > > > > The old device property API (device_add_properties()) is going to be > > removed. These prepare the i2c subsystem and drivers for the change. > > The change is fairly trivial in case of i2c. All we need to do is add > > complete software nodes to the devices instead of only the device > > properties in those nodes. > > > > thanks, > > > > Heikki Krogerus (12): > > i2c: Add support for software nodes > > ARM: davinci: Constify the software nodes > > ARM: omap1: osk: Constify the software node > > ARM: pxa: stargate2: Constify the software node > > ARM: s3c: mini2440: Constify the software node > > platform/x86: intel_cht_int33fe_microb: Constify the software node > > i2c: cht-wc: Constify the software node > > i2c: nvidia-gpu: Constify the software node > > i2c: icy: Constify the software node > > platform/chrome: chromeos_laptop - Prepare complete software nodes > > Input: elantech - Prepare a complete software node for the device > > i2c: Remove support for dangling device properties > > I applied all patches to an immutable branch "i2c/software-nodes" now. > Once buildbot successfully checked this branch, I will merge it into > for-next and advertise it to interested parties. Thank you! -- heikki
On Mon, Mar 29, 2021 at 01:50:35PM +0300, Heikki Krogerus wrote: > Hi, > > The old device property API (device_add_properties()) is going to be > removed. These prepare the i2c subsystem and drivers for the change. > The change is fairly trivial in case of i2c. All we need to do is add > complete software nodes to the devices instead of only the device > properties in those nodes. > > thanks, > > Heikki Krogerus (12): > i2c: Add support for software nodes > ARM: davinci: Constify the software nodes > ARM: omap1: osk: Constify the software node > ARM: pxa: stargate2: Constify the software node > ARM: s3c: mini2440: Constify the software node > platform/x86: intel_cht_int33fe_microb: Constify the software node > i2c: cht-wc: Constify the software node > i2c: nvidia-gpu: Constify the software node > i2c: icy: Constify the software node > platform/chrome: chromeos_laptop - Prepare complete software nodes > Input: elantech - Prepare a complete software node for the device > i2c: Remove support for dangling device properties Merged the immutable branch (with added tag from Robert) to for-next now. Branch is here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/software-nodes tag: <i2c-software-nodes-20210410> Thank you, Heikki and all reviewers!