| Message ID | 20230723083721.35384-2-biju.das.jz@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | Extend device_get_match_data() to struct bus_type | expand |
On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: Thank you for your contribution! My comments below. > Extend device_get_match_data() to buses (for eg: I2C) by adding a > callback device_get_match_data() to struct bus_type() and call this method > as a fallback for generic fwnode based device_get_match_data(). > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> You can't just throw one's SoB tag without clear understanding what's going on here (either wrong authorship or missing Co-developed-by or...?). > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> ... > const void *device_get_match_data(const struct device *dev) > { > - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); > + const void *data; > + > + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); > + if (!data && dev->bus && dev->bus->get_match_data) > + data = dev->bus->get_match_data(dev); > + > + return data; Much better looking is data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); if (data) return data; if (dev->bus && dev->bus->get_match_data) return dev->bus->get_match_data(dev); return NULL; > }
On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote: > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: > > Thank you for your contribution! > My comments below. > > > Extend device_get_match_data() to buses (for eg: I2C) by adding a > > callback device_get_match_data() to struct bus_type() and call this method > > as a fallback for generic fwnode based device_get_match_data(). > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > You can't just throw one's SoB tag without clear understanding what's going on > here (either wrong authorship or missing Co-developed-by or...?). > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> ... > > const void *device_get_match_data(const struct device *dev) Btw, this needs a documentation update to explain how it works now. > > { > > - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); > > + const void *data; > > + > > + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); > > + if (!data && dev->bus && dev->bus->get_match_data) > > + data = dev->bus->get_match_data(dev); > > + > > + return data; > > Much better looking is > > data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); > if (data) > return data; > > if (dev->bus && dev->bus->get_match_data) > return dev->bus->get_match_data(dev); > > return NULL; > > > }
Hi Andy, Thanks for the feedback. > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote: > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: > > > > Thank you for your contribution! > > My comments below. > > > > > Extend device_get_match_data() to buses (for eg: I2C) by adding a > > > callback device_get_match_data() to struct bus_type() and call this > > > method as a fallback for generic fwnode based > device_get_match_data(). > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > You can't just throw one's SoB tag without clear understanding what's > > going on here (either wrong authorship or missing Co-developed-by > or...?). > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > ... > > > > const void *device_get_match_data(const struct device *dev) > > Btw, this needs a documentation update to explain how it works now. Can you please point me to the location where I need to update? Cheers, Biju > > > > { > > > - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > > > + const void *data; > > > + > > > + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > > > + if (!data && dev->bus && dev->bus->get_match_data) > > > + data = dev->bus->get_match_data(dev); > > > + > > > + return data; > > > > Much better looking is > > > > data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > > if (data) > > return data; > > > > if (dev->bus && dev->bus->get_match_data) > > return dev->bus->get_match_data(dev); > > > > return NULL; > > > > > } > > -- > With Best Regards, > Andy Shevchenko >
Hi Andy Shevchenko, Thanks for the feedback. > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: > > Thank you for your contribution! > My comments below. > > > Extend device_get_match_data() to buses (for eg: I2C) by adding a > > callback device_get_match_data() to struct bus_type() and call this > > method as a fallback for generic fwnode based device_get_match_data(). > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > You can't just throw one's SoB tag without clear understanding what's > going on here (either wrong authorship or missing Co-developed-by > or...?). Dmitry feels instead of having separate bus based match_data() like i2c_get_match_data[2] and spi_get_device_match_data[3], it is better to have a generic approach like a single API device_get_match_data() for getting match_data for OF/ACPI/I2C/SPI tables. So, he came with a proposal and shared some code here[1]. Since,I have send this patch, I put my signed -off. If this patch is accepted, then we can get rid of bus based match_data. [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25436207 [2] https://elixir.bootlin.com/linux/v6.5-rc3/source/drivers/i2c/i2c-core-base.c#L117 [3] https://elixir.bootlin.com/linux/v6.5-rc3/source/drivers/spi/spi.c#L364 Cheers, Biju > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > ... > > > const void *device_get_match_data(const struct device *dev) { > > - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > > + const void *data; > > + > > + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > > + if (!data && dev->bus && dev->bus->get_match_data) > > + data = dev->bus->get_match_data(dev); > > + > > + return data; > > Much better looking is > > data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, > dev); > if (data) > return data; OK. Cheers, Biju > > if (dev->bus && dev->bus->get_match_data) > return dev->bus->get_match_data(dev); > > return NULL; > > > } > > -- > With Best Regards, > Andy Shevchenko >
On Mon, Jul 24, 2023 at 11:46:44AM +0000, Biju Das wrote: > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 02:06:07PM +0300, Andy Shevchenko wrote: > > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: ... > > > > const void *device_get_match_data(const struct device *dev) (1) > > Btw, this needs a documentation update to explain how it works now. > > Can you please point me to the location where I need to update? Sure. It's just on top of the (1). It looks like no documentation yet existed, so you need to create one. Not sure if fwnode.h has to be updated. At least it doesn't contradict with the added code, while not describing all details.
On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote: > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > device_get_match_data() to struct bus_type > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: ... > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > You can't just throw one's SoB tag without clear understanding what's > > going on here (either wrong authorship or missing Co-developed-by > > or...?). > > Dmitry feels instead of having separate bus based match_data() like > i2c_get_match_data[2] and spi_get_device_match_data[3], it is better to have > a generic approach like a single API device_get_match_data() for getting > match_data for OF/ACPI/I2C/SPI tables. > > So, he came with a proposal and shared some code here[1]. Yes, I'm pretty much following the discussion. > Since,I have send this patch, I put my signed -off. I'm not talking about this. There is no evidence that Dmitry gives you any approval to use or clear SoB tag. Again, you may not do like this. > If this patch is accepted, then we can get rid of bus based match_data. This is unrelated to what I talking about. > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25436207
Hi Andy, Thanks for the feedback. > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote: > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 at > > > 09:37:20AM +0100, Biju Das wrote: > > ... > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > > > You can't just throw one's SoB tag without clear understanding > > > what's going on here (either wrong authorship or missing > > > Co-developed-by or...?). > > > > Dmitry feels instead of having separate bus based match_data() like > > i2c_get_match_data[2] and spi_get_device_match_data[3], it is better > > to have a generic approach like a single API device_get_match_data() > > for getting match_data for OF/ACPI/I2C/SPI tables. > > > > So, he came with a proposal and shared some code here[1]. > > Yes, I'm pretty much following the discussion. > > > Since,I have send this patch, I put my signed -off. > > I'm not talking about this. There is no evidence that Dmitry gives you > any approval to use or clear SoB tag. Again, you may not do like this. Here Dmitry is acknowledging, he is ok with the patch I posted. https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25437032 Cheers, Biju
Hi Andy Shevchenko, Thanks for the feedback. > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 11:46:44AM +0000, Biju Das wrote: > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at > > > 02:06:07PM +0300, Andy Shevchenko wrote: > > > > On Sun, Jul 23, 2023 at 09:37:20AM +0100, Biju Das wrote: > > ... > > > > > > const void *device_get_match_data(const struct device *dev) > > (1) > > > > Btw, this needs a documentation update to explain how it works now. > > > > Can you please point me to the location where I need to update? > > Sure. It's just on top of the (1). It looks like no documentation yet > existed, so you need to create one. > > Not sure if fwnode.h has to be updated. At least it doesn't contradict > with the added code, while not describing all details. OK, will do. Cheers, Biju
On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote: > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 12:02:27PM +0000, Biju Das wrote: > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 at > > > > 09:37:20AM +0100, Biju Das wrote: ... > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > > > > > You can't just throw one's SoB tag without clear understanding > > > > what's going on here (either wrong authorship or missing > > > > Co-developed-by or...?). > > > > > > Dmitry feels instead of having separate bus based match_data() like > > > i2c_get_match_data[2] and spi_get_device_match_data[3], it is better > > > to have a generic approach like a single API device_get_match_data() > > > for getting match_data for OF/ACPI/I2C/SPI tables. > > > > > > So, he came with a proposal and shared some code here[1]. > > > > Yes, I'm pretty much following the discussion. > > > > > Since,I have send this patch, I put my signed -off. > > > > I'm not talking about this. There is no evidence that Dmitry gives you > > any approval to use or clear SoB tag. Again, you may not do like this. > > Here Dmitry is acknowledging, he is ok with the patch I posted. > > https://patchwork.kernel.org/project/linux-renesas-soc/patch/20230717131756.240645-2-biju.das.jz@bp.renesas.com/#25437032 No, you just misinterpreted his message. See https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin for the explanation. The SoB has to be explicitly given. Dmitry had _not_ put it like this.
Hi Andy, Thanks for the feedback. > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote: > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at > > > 12:02:27PM +0000, Biju Das wrote: > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 > > > > > at 09:37:20AM +0100, Biju Das wrote: > > ... > > > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > > > > > > > You can't just throw one's SoB tag without clear understanding > > > > > what's going on here (either wrong authorship or missing > > > > > Co-developed-by or...?). > > > > > > > > Dmitry feels instead of having separate bus based match_data() > > > > like i2c_get_match_data[2] and spi_get_device_match_data[3], it is > > > > better to have a generic approach like a single API > > > > device_get_match_data() for getting match_data for OF/ACPI/I2C/SPI > tables. > > > > > > > > So, he came with a proposal and shared some code here[1]. > > > > > > Yes, I'm pretty much following the discussion. > > > > > > > Since,I have send this patch, I put my signed -off. > > > > > > I'm not talking about this. There is no evidence that Dmitry gives > > > you any approval to use or clear SoB tag. Again, you may not do like > this. > > > > Here Dmitry is acknowledging, he is ok with the patch I posted. > > > > No, you just misinterpreted his message. > Dmitry, As you are the author of code, either you post a patch or provide your SoB as per the guideline mentioned here to avoid confusion. https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin Cheers, Biju
On Mon, Jul 24, 2023 at 01:58:55PM +0000, Biju Das wrote: > Hi Andy, > > Thanks for the feedback. > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > device_get_match_data() to struct bus_type > > > > On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote: > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 at > > > > 12:02:27PM +0000, Biju Das wrote: > > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, 2023 > > > > > > at 09:37:20AM +0100, Biju Das wrote: > > > > ... > > > > > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > > > > > > > > > You can't just throw one's SoB tag without clear understanding > > > > > > what's going on here (either wrong authorship or missing > > > > > > Co-developed-by or...?). > > > > > > > > > > Dmitry feels instead of having separate bus based match_data() > > > > > like i2c_get_match_data[2] and spi_get_device_match_data[3], it is > > > > > better to have a generic approach like a single API > > > > > device_get_match_data() for getting match_data for OF/ACPI/I2C/SPI > > tables. > > > > > > > > > > So, he came with a proposal and shared some code here[1]. > > > > > > > > Yes, I'm pretty much following the discussion. > > > > > > > > > Since,I have send this patch, I put my signed -off. > > > > > > > > I'm not talking about this. There is no evidence that Dmitry gives > > > > you any approval to use or clear SoB tag. Again, you may not do like > > this. > > > > > > Here Dmitry is acknowledging, he is ok with the patch I posted. > > > > > > > No, you just misinterpreted his message. > > > > Dmitry, > > As you are the author of code, either you post a patch or provide your SoB as per the guideline mentioned here to avoid confusion. > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin It was not really proper patch, consider it as an email with parts written in unified diff, as sometimes it is easier than to explain in words, and I do not want to take much credit for it. If you wish you can put "Suggested-by" for me, or just drop my name off the patch description altogether. Thanks.
> Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > device_get_match_data() to struct bus_type > > On Mon, Jul 24, 2023 at 01:58:55PM +0000, Biju Das wrote: > > Hi Andy, > > > > Thanks for the feedback. > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > device_get_match_data() to struct bus_type > > > > > > On Mon, Jul 24, 2023 at 01:19:02PM +0000, Biju Das wrote: > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > > device_get_match_data() to struct bus_type On Mon, Jul 24, 2023 > > > > > at 12:02:27PM +0000, Biju Das wrote: > > > > > > > Subject: Re: [PATCH RFC 1/2] drivers: fwnode: Extend > > > > > > > device_get_match_data() to struct bus_type On Sun, Jul 23, > > > > > > > 2023 at 09:37:20AM +0100, Biju Das wrote: > > > > > > ... > > > > > > > > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > > > > > > > > > > > You can't just throw one's SoB tag without clear > > > > > > > understanding what's going on here (either wrong authorship > > > > > > > or missing Co-developed-by or...?). > > > > > > > > > > > > Dmitry feels instead of having separate bus based match_data() > > > > > > like i2c_get_match_data[2] and spi_get_device_match_data[3], > > > > > > it is better to have a generic approach like a single API > > > > > > device_get_match_data() for getting match_data for > > > > > > OF/ACPI/I2C/SPI > > > tables. > > > > > > > > > > > > So, he came with a proposal and shared some code here[1]. > > > > > > > > > > Yes, I'm pretty much following the discussion. > > > > > > > > > > > Since,I have send this patch, I put my signed -off. > > > > > > > > > > I'm not talking about this. There is no evidence that Dmitry > > > > > gives you any approval to use or clear SoB tag. Again, you may > > > > > not do like > > > this. > > > > > > > > Here Dmitry is acknowledging, he is ok with the patch I posted. > > > > > > > > > > No, you just misinterpreted his message. > > > > > > > Dmitry, > > > > As you are the author of code, either you post a patch or provide your > SoB as per the guideline mentioned here to avoid confusion. > > > > > It was not really proper patch, consider it as an email with parts > written in unified diff, as sometimes it is easier than to explain in > words, and I do not want to take much credit for it. > > If you wish you can put "Suggested-by" for me, or just drop my name off > the patch description altogether. Sure, will add Suggested-by tag. Cheers, Biju
diff --git a/drivers/base/property.c b/drivers/base/property.c index 8c40abed7852..cc0bf7bb6f3a 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -1277,7 +1277,13 @@ EXPORT_SYMBOL(fwnode_graph_parse_endpoint); const void *device_get_match_data(const struct device *dev) { - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); + const void *data; + + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); + if (!data && dev->bus && dev->bus->get_match_data) + data = dev->bus->get_match_data(dev); + + return data; } EXPORT_SYMBOL_GPL(device_get_match_data); diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h index ae10c4322754..2e15b0ae5384 100644 --- a/include/linux/device/bus.h +++ b/include/linux/device/bus.h @@ -60,6 +60,7 @@ struct fwnode_handle; * this bus. * @dma_cleanup: Called to cleanup DMA configuration on a device on * this bus. + * @get_match_data: Called to get match data on a device on this bus. * @pm: Power management operations of this bus, callback the specific * device driver's pm-ops. * @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU @@ -102,6 +103,8 @@ struct bus_type { int (*dma_configure)(struct device *dev); void (*dma_cleanup)(struct device *dev); + const void *(*get_match_data)(const struct device *dev); + const struct dev_pm_ops *pm; const struct iommu_ops *iommu_ops;