1. Patchew
  2. linux
  3. [PATCH v6 0/5] drm/xe: i2c support
  4. View patch

[PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource

Heikki Krogerus posted 5 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource
Posted by Heikki Krogerus 3 months, 1 week ago 
The irq resource itself can be used as a generic way to
determine when polling is needed.

This not only removes the need for special additional device
properties that would soon be needed when the platform may
or may not have the irq, but it also removes the need to
check the platform in the first place in order to determine
is polling needed or not.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Hi guys,

I found the thread with Jarkko's comments from my archives. He wanted
the local flags variable to be added because he wants the order of the
calls to remain as it is now - the device is allocated only after the
irq is checked.

thanks,
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 879719e91df2..3104f52e32be 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -205,25 +205,28 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
 
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
+	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
 	struct device *device = &pdev->dev;
 	struct i2c_adapter *adap;
 	struct dw_i2c_dev *dev;
 	int irq, ret;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq == -ENXIO)
+		flags |= ACCESS_POLLING;
+	else if (irq < 0)
 		return irq;
 
 	dev = devm_kzalloc(device, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
-	dev->flags = (uintptr_t)device_get_match_data(device);
 	if (device_property_present(device, "wx,i2c-snps-model"))
-		dev->flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
+		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
 
 	dev->dev = device;
 	dev->irq = irq;
+	dev->flags = flags;
 	platform_set_drvdata(pdev, dev);
 
 	ret = dw_i2c_plat_request_regs(dev);
-- 
2.47.2
Re: [PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource
Posted by Andi Shyti 3 months, 1 week ago 
Hi Heikki,

On Tue, Jul 01, 2025 at 03:22:48PM +0300, Heikki Krogerus wrote:
> The irq resource itself can be used as a generic way to
> determine when polling is needed.
> 
> This not only removes the need for special additional device
> properties that would soon be needed when the platform may
> or may not have the irq, but it also removes the need to
> check the platform in the first place in order to determine
> is polling needed or not.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi
Re: [PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource
Posted by Andy Shevchenko 3 months, 1 week ago 
On Tue, Jul 01, 2025 at 03:22:48PM +0300, Heikki Krogerus wrote:
> The irq resource itself can be used as a generic way to
> determine when polling is needed.
> 
> This not only removes the need for special additional device
> properties that would soon be needed when the platform may
> or may not have the irq, but it also removes the need to
> check the platform in the first place in order to determine
> is polling needed or not.

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> Hi guys,
> 
> I found the thread with Jarkko's comments from my archives. He wanted
> the local flags variable to be added because he wants the order of the
> calls to remain as it is now - the device is allocated only after the
> irq is checked.

Yes, thanks.

...

> +	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);

> +	irq = platform_get_irq_optional(pdev, 0);
> +	if (irq == -ENXIO)
> +		flags |= ACCESS_POLLING;
> +	else if (irq < 0)
>  		return irq;

>  	if (device_property_present(device, "wx,i2c-snps-model"))
> +		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;

Now I'm a bit puzzled why do we need to add this flag explicitly here?
Does Wnagxun provides an IRQ and chooses at the same time to poll?
Shouldn't this patch rather fix that?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource
Posted by Heikki Krogerus 3 months, 1 week ago 
On Tue, Jul 01, 2025 at 03:42:42PM +0300, Andy Shevchenko wrote:
> On Tue, Jul 01, 2025 at 03:22:48PM +0300, Heikki Krogerus wrote:
> > The irq resource itself can be used as a generic way to
> > determine when polling is needed.
> > 
> > This not only removes the need for special additional device
> > properties that would soon be needed when the platform may
> > or may not have the irq, but it also removes the need to
> > check the platform in the first place in order to determine
> > is polling needed or not.
> 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > Hi guys,
> > 
> > I found the thread with Jarkko's comments from my archives. He wanted
> > the local flags variable to be added because he wants the order of the
> > calls to remain as it is now - the device is allocated only after the
> > irq is checked.
> 
> Yes, thanks.
> 
> ...
> 
> > +	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
> 
> > +	irq = platform_get_irq_optional(pdev, 0);
> > +	if (irq == -ENXIO)
> > +		flags |= ACCESS_POLLING;
> > +	else if (irq < 0)
> >  		return irq;
> 
> >  	if (device_property_present(device, "wx,i2c-snps-model"))
> > +		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
> 
> Now I'm a bit puzzled why do we need to add this flag explicitly here?
> Does Wnagxun provides an IRQ and chooses at the same time to poll?
> Shouldn't this patch rather fix that?

No. I do not want to touch the behavior here. The flags were
overwritten and continue to be overwritten.

I will propose an improvement for that together with some other
modifications to this file later, but those are out side the scope of
this series.

thanks,

-- 
heikki
Re: [PATCH v6 1/5] i2c: designware: Use polling by default when there is no irq resource
Posted by Andy Shevchenko 3 months, 1 week ago 
On Tue, Jul 01, 2025 at 03:55:08PM +0300, Heikki Krogerus wrote:
> On Tue, Jul 01, 2025 at 03:42:42PM +0300, Andy Shevchenko wrote:
> > On Tue, Jul 01, 2025 at 03:22:48PM +0300, Heikki Krogerus wrote:

...

> > > +	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
> > 
> > > +	irq = platform_get_irq_optional(pdev, 0);
> > > +	if (irq == -ENXIO)
> > > +		flags |= ACCESS_POLLING;
> > > +	else if (irq < 0)
> > >  		return irq;
> > 
> > >  	if (device_property_present(device, "wx,i2c-snps-model"))
> > > +		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
> > 
> > Now I'm a bit puzzled why do we need to add this flag explicitly here?
> > Does Wnagxun provides an IRQ and chooses at the same time to poll?
> > Shouldn't this patch rather fix that?
> 
> No. I do not want to touch the behavior here. The flags were
> overwritten and continue to be overwritten.
> 
> I will propose an improvement for that together with some other
> modifications to this file later, but those are out side the scope of
> this series.

Sure.

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.