All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
@ 2021-06-03 16:58 Jonathan Cameron
  2021-06-03 19:55 ` Wolfram Sang
  0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2021-06-03 16:58 UTC (permalink / raw)
  To: linux-iio; +Cc: Jonathan Cameron, Tom Rix, Sean Nyekjaer, Wolfram Sang

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

If I2C is not compiled, there is no way we should see a call to
i2c_verify_client() on a device that is an i2c client. As such,
provide a stub to return NULL to resolve an associated build failure.

The build is failing with this link error
ld: fxls8962af-core.o: in function `fxls8962af_fifo_transfer':
fxls8962af-core.c: undefined reference to `i2c_verify_client'

Reported-by: Tom Rix <trix@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
Reviewed-by: Sean Nyekjaer <sean@geanix.com>
Cc: Wolfram Sang <wsa@kernel.org>
---

V2: Move to end of existing #if IS_ENABLED(CONFIG_I2C) block.

Plan for this an immutable branch in iio.git which can be pulled into both
the togreg branch of iio.git and the i2c tree.
Fixes tag is there so that people can track the need for this patch if
they are backporting that device errata work around.

 include/linux/i2c.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e8f2ac8c9c3d..7d71131c394e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -343,7 +343,6 @@ struct i2c_client {
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
 
-struct i2c_client *i2c_verify_client(struct device *dev);
 struct i2c_adapter *i2c_verify_adapter(struct device *dev);
 const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 					 const struct i2c_client *client);
@@ -477,6 +476,13 @@ i2c_new_ancillary_device(struct i2c_client *client,
 			 u16 default_addr);
 
 void i2c_unregister_device(struct i2c_client *client);
+
+struct i2c_client *i2c_verify_client(struct device *dev);
+#else
+static inline struct i2c_client *i2c_verify_client(struct device *dev)
+{
+	return NULL;
+}
 #endif /* I2C */
 
 /* Mainboard arch_initcall() code should register all its I2C devices.
-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-06-03 16:58 [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C Jonathan Cameron
@ 2021-06-03 19:55 ` Wolfram Sang
  2021-06-08 16:22   ` Jonathan Cameron
  0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2021-06-03 19:55 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Jonathan Cameron, Tom Rix, Sean Nyekjaer

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Thu, Jun 03, 2021 at 05:58:35PM +0100, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> If I2C is not compiled, there is no way we should see a call to
> i2c_verify_client() on a device that is an i2c client. As such,
> provide a stub to return NULL to resolve an associated build failure.
> 
> The build is failing with this link error
> ld: fxls8962af-core.o: in function `fxls8962af_fifo_transfer':
> fxls8962af-core.c: undefined reference to `i2c_verify_client'
> 
> Reported-by: Tom Rix <trix@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
> Reviewed-by: Sean Nyekjaer <sean@geanix.com>
> Cc: Wolfram Sang <wsa@kernel.org>

Thanks for doing this!

Acked-by: Wolfram Sang <wsa@kernel.org>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-06-03 19:55 ` Wolfram Sang
@ 2021-06-08 16:22   ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2021-06-08 16:22 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-iio, Jonathan Cameron, Tom Rix, Sean Nyekjaer

On Thu, 3 Jun 2021 21:55:51 +0200
Wolfram Sang <wsa@kernel.org> wrote:

> On Thu, Jun 03, 2021 at 05:58:35PM +0100, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > If I2C is not compiled, there is no way we should see a call to
> > i2c_verify_client() on a device that is an i2c client. As such,
> > provide a stub to return NULL to resolve an associated build failure.
> > 
> > The build is failing with this link error
> > ld: fxls8962af-core.o: in function `fxls8962af_fifo_transfer':
> > fxls8962af-core.c: undefined reference to `i2c_verify_client'
> > 
> > Reported-by: Tom Rix <trix@redhat.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
> > Reviewed-by: Sean Nyekjaer <sean@geanix.com>
> > Cc: Wolfram Sang <wsa@kernel.org>  
> 
> Thanks for doing this!
> 
> Acked-by: Wolfram Sang <wsa@kernel.org>
> 

immutable branch at:
ib-iio-i2c-5.13rc1 https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

I let this sit in next (via a merge with IIO togreg branch) for a day to
shake out any ways in which I messed it up.

As it turns out, that was a good plan given the fixes tag had gone stale.
Now fixed that and will be sending a pull request to Greg KH including the
update IIO merge shortly (which is in linux-next today).

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-06-03 11:55     ` Wolfram Sang
@ 2021-06-03 12:42       ` Jonathan Cameron
  0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2021-06-03 12:42 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jonathan Cameron, linux-iio, linux-i2c, Tom Rix, Sean Nyekjaer

On Thu, 3 Jun 2021 13:55:59 +0200
Wolfram Sang <wsa@kernel.org> wrote:

> Hi Jonathan
> 
> > Ok, so that is there because my assumption was that mostly like I'd take
> > this patch through IIO, in which case it's directly valid and necessary
> > for backport information purposes.  I'm guessing this one is unlikely to
> > cause merge conflicts given how localized it is...  
> 
> I see. Makes sense.
> 
> > You would do an immutable branch that I can pull into IIO. I'd really like
> > to avoid rebasing the IIO tree unless absolutely necessary as people are
> > working on top if it.  
> 
> Sure, let's avoid rebasing.
> 
> > Doesn't work.  There is a high chance the original patch will get ported
> > back to earlier kernels and there is no reference to let anyone know they
> > also need this one to avoid potential build issues on the stable kernel.
> > 
> > So, if you want to take this through I2C, the path forwards would be.
> > 1) You take this one through I2C
> > 2) I apply the original fix (which #ifdefs the relevant code out in the
> >    driver).
> > 3) Once (1) is in mainline next cycle, I can revert (2) on the basis
> >    it is no longer necessary.
> > 
> > I'm fine with doing it this way as it avoids any cross dependencies.  
> 
> The other solution is that you make an immutable branch for me? IIUC,
> this would be easiest? It would work for me.

Sure, I'll do that once we've agreed a v2

> 
> > > > +#if IS_ENABLED(CONFIG_I2C)    
> > > 
> > > Hmm, can't we move this into an already existing IS_ENABLED block?  
> > 
> > There aren't any similar #if / #else blocks for CONFIG_I2C in i2c.h
> > so it seemed neater to just add one around this individual element
> > and not destroy the general organization of the file.  
> 
> Could be argued. I'd still prefer to add it at line 480 (5.13-rc3) with
> the #else branch added if you don't mind.

Sure, I'll move it.

Jonathan

> 
> Thanks and kind regards,
> 
>    Wolfram
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-06-03 11:24   ` Jonathan Cameron
@ 2021-06-03 11:55     ` Wolfram Sang
  2021-06-03 12:42       ` Jonathan Cameron
  0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2021-06-03 11:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jonathan Cameron, linux-iio, linux-i2c, Tom Rix, Sean Nyekjaer

[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]

Hi Jonathan

> Ok, so that is there because my assumption was that mostly like I'd take
> this patch through IIO, in which case it's directly valid and necessary
> for backport information purposes.  I'm guessing this one is unlikely to
> cause merge conflicts given how localized it is...

I see. Makes sense.

> You would do an immutable branch that I can pull into IIO. I'd really like
> to avoid rebasing the IIO tree unless absolutely necessary as people are
> working on top if it.

Sure, let's avoid rebasing.

> Doesn't work.  There is a high chance the original patch will get ported
> back to earlier kernels and there is no reference to let anyone know they
> also need this one to avoid potential build issues on the stable kernel.
> 
> So, if you want to take this through I2C, the path forwards would be.
> 1) You take this one through I2C
> 2) I apply the original fix (which #ifdefs the relevant code out in the
>    driver).
> 3) Once (1) is in mainline next cycle, I can revert (2) on the basis
>    it is no longer necessary.
> 
> I'm fine with doing it this way as it avoids any cross dependencies.

The other solution is that you make an immutable branch for me? IIUC,
this would be easiest? It would work for me.

> > > +#if IS_ENABLED(CONFIG_I2C)  
> > 
> > Hmm, can't we move this into an already existing IS_ENABLED block?
> 
> There aren't any similar #if / #else blocks for CONFIG_I2C in i2c.h
> so it seemed neater to just add one around this individual element
> and not destroy the general organization of the file.

Could be argued. I'd still prefer to add it at line 480 (5.13-rc3) with
the #else branch added if you don't mind.

Thanks and kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-05-27 20:23 ` Wolfram Sang
@ 2021-06-03 11:24   ` Jonathan Cameron
  2021-06-03 11:55     ` Wolfram Sang
  0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2021-06-03 11:24 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jonathan Cameron, linux-iio, linux-i2c, Tom Rix, Sean Nyekjaer

On Thu, 27 May 2021 22:23:34 +0200
Wolfram Sang <wsa@kernel.org> wrote:

> Hi Jonathan,
> 
> > Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
> > Cc: Sean Nyekjaer <sean@geanix.com>
> > Cc: Wolfram Sang <wsa@kernel.org>
> > ---
> > 
> > Note the broken patch is only in the IIO/togreg branch at the moment.  
> 
> Then the fixes tag should be removed. It is only for upstream commits.

Ok, so that is there because my assumption was that mostly like I'd take
this patch through IIO, in which case it's directly valid and necessary
for backport information purposes.  I'm guessing this one is unlikely to
cause merge conflicts given how localized it is...

You would do an immutable branch that I can pull into IIO. I'd really like
to avoid rebasing the IIO tree unless absolutely necessary as people are
working on top if it.
 
> It means we will have a merge dependency the next cycle, so I will send
> my pull request early.

Doesn't work.  There is a high chance the original patch will get ported
back to earlier kernels and there is no reference to let anyone know they
also need this one to avoid potential build issues on the stable kernel.

So, if you want to take this through I2C, the path forwards would be.
1) You take this one through I2C
2) I apply the original fix (which #ifdefs the relevant code out in the
   driver).
3) Once (1) is in mainline next cycle, I can revert (2) on the basis
   it is no longer necessary.

I'm fine with doing it this way as it avoids any cross dependencies.

> 
> > 
> >  include/linux/i2c.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index e8f2ac8c9c3d..aa52738b9c46 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -343,7 +343,14 @@ struct i2c_client {
> >  };
> >  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
> >  
> > +#if IS_ENABLED(CONFIG_I2C)  
> 
> Hmm, can't we move this into an already existing IS_ENABLED block?

There aren't any similar #if / #else blocks for CONFIG_I2C in i2c.h
so it seemed neater to just add one around this individual element
and not destroy the general organization of the file.

Jonathan



> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-05-26 17:44 Jonathan Cameron
  2021-05-27  5:38 ` Sean Nyekjaer
@ 2021-05-27 20:23 ` Wolfram Sang
  2021-06-03 11:24   ` Jonathan Cameron
  1 sibling, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2021-05-27 20:23 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, linux-i2c, Jonathan Cameron, Tom Rix, Sean Nyekjaer

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

Hi Jonathan,

> Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
> Cc: Sean Nyekjaer <sean@geanix.com>
> Cc: Wolfram Sang <wsa@kernel.org>
> ---
> 
> Note the broken patch is only in the IIO/togreg branch at the moment.

Then the fixes tag should be removed. It is only for upstream commits.
It means we will have a merge dependency the next cycle, so I will send
my pull request early.

> 
>  include/linux/i2c.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index e8f2ac8c9c3d..aa52738b9c46 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -343,7 +343,14 @@ struct i2c_client {
>  };
>  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
>  
> +#if IS_ENABLED(CONFIG_I2C)

Hmm, can't we move this into an already existing IS_ENABLED block?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
  2021-05-26 17:44 Jonathan Cameron
@ 2021-05-27  5:38 ` Sean Nyekjaer
  2021-05-27 20:23 ` Wolfram Sang
  1 sibling, 0 replies; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-27  5:38 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio, linux-i2c
  Cc: Jonathan Cameron, Tom Rix, Wolfram Sang

On 26/05/2021 19.44, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> If I2C is not compiled, there is no way we should see a call to
> i2c_verify_client() on a device that is an i2c client. As such,
> provide a stub to return NULL to resolve an associated build failure.
> 
> The build is failing with this link error
> ld: fxls8962af-core.o: in function `fxls8962af_fifo_transfer':
> fxls8962af-core.c: undefined reference to `i2c_verify_client'
> 
> Reported-by: Tom Rix <trix@redhat.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
> Cc: Sean Nyekjaer <sean@geanix.com>
> Cc: Wolfram Sang <wsa@kernel.org>
Reviewed-by: Sean Nyekjaer <sean@geanix.com>
> ---
> 
> Note the broken patch is only in the IIO/togreg branch at the moment.

Didn't quite get that the stub was should go in linux/i2c.h, thought I had to make a stub in the driver.
But this seems appropriate :)

/Sean

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C
@ 2021-05-26 17:44 Jonathan Cameron
  2021-05-27  5:38 ` Sean Nyekjaer
  2021-05-27 20:23 ` Wolfram Sang
  0 siblings, 2 replies; 9+ messages in thread
From: Jonathan Cameron @ 2021-05-26 17:44 UTC (permalink / raw)
  To: linux-iio, linux-i2c
  Cc: Jonathan Cameron, Tom Rix, Sean Nyekjaer, Wolfram Sang

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

If I2C is not compiled, there is no way we should see a call to
i2c_verify_client() on a device that is an i2c client. As such,
provide a stub to return NULL to resolve an associated build failure.

The build is failing with this link error
ld: fxls8962af-core.o: in function `fxls8962af_fifo_transfer':
fxls8962af-core.c: undefined reference to `i2c_verify_client'

Reported-by: Tom Rix <trix@redhat.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Fixes: 68068fad0e1c ("iio: accel: fxls8962af: fix errata bug E3 - I2C burst reads")
Cc: Sean Nyekjaer <sean@geanix.com>
Cc: Wolfram Sang <wsa@kernel.org>
---

Note the broken patch is only in the IIO/togreg branch at the moment.

 include/linux/i2c.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e8f2ac8c9c3d..aa52738b9c46 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -343,7 +343,14 @@ struct i2c_client {
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
 
+#if IS_ENABLED(CONFIG_I2C)
 struct i2c_client *i2c_verify_client(struct device *dev);
+#else
+static inline struct i2c_client *i2c_verify_client(struct device *dev)
+{
+	return NULL;
+}
+#endif
 struct i2c_adapter *i2c_verify_adapter(struct device *dev);
 const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
 					 const struct i2c_client *client);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-08 16:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 16:58 [PATCH] i2c: core: Add stub for i2c_verify_client() if !CONFIG_I2C Jonathan Cameron
2021-06-03 19:55 ` Wolfram Sang
2021-06-08 16:22   ` Jonathan Cameron
  -- strict thread matches above, loose matches on Subject: below --
2021-05-26 17:44 Jonathan Cameron
2021-05-27  5:38 ` Sean Nyekjaer
2021-05-27 20:23 ` Wolfram Sang
2021-06-03 11:24   ` Jonathan Cameron
2021-06-03 11:55     ` Wolfram Sang
2021-06-03 12:42       ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.