Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[George Cherian]

On 03/06/2018 02:48 PM, Phil Reid wrote:
> On 6/03/2018 16:36, Jan Glauber wrote:
> > On Tue, Feb 27, 2018 at 01:26:20PM +0000, George Cherian wrote:
> > > Add support for SMBus alert mechanism to i2c-xlp9xx driver.
> > > The second interrupt is parsed to use for SMBus alert.
> > > The first interrupt is the i2c controller main interrupt.
> > >
> > > Signed-off-by: Kamlakant Patel <kamlakant.patel@xxxxxxxxxx>
> > > Signed-off-by: George Cherian <george.cherian@xxxxxxxxxx>
> > > ---
> > > Â drivers/i2c/busses/i2c-xlp9xx.c | 26 ++++++++++++++++++++++++++
> > > Â 1 file changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-xlp9xx.c 
> > > b/drivers/i2c/busses/i2c-xlp9xx.c
> > > index eb8913e..9462eab 100644
> > > --- a/drivers/i2c/busses/i2c-xlp9xx.c
> > > +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> > > @@ -10,6 +10,7 @@
> > > Â #include <linux/clk.h>
> > > Â #include <linux/completion.h>
> > > Â #include <linux/i2c.h>
> > > +#include <linux/i2c-smbus.h>
> > > Â #include <linux/init.h>
> > > Â #include <linux/interrupt.h>
> > > Â #include <linux/io.h>
> > > @@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
> > > ÂÂÂÂÂ struct device *dev;
> > > ÂÂÂÂÂ struct i2c_adapter adapter;
> > > ÂÂÂÂÂ struct completion msg_complete;
> > > +ÂÂÂ struct i2c_smbus_alert_setup alert_data;
> > > +ÂÂÂ struct i2c_client *ara;
> > > ÂÂÂÂÂ int irq;
> > > ÂÂÂÂÂ bool msg_read;
> > > ÂÂÂÂÂ bool len_recv;
> > > @@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct 
> > > platform_device *pdev,
> > > ÂÂÂÂÂ return 0;
> > > Â }
> > > +static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct platform_device *pdev)
> > > +{
> > > +ÂÂÂ if (!priv->alert_data.irq)
> > > +ÂÂÂÂÂÂÂ return -EINVAL;
> > > +
> > > +ÂÂÂ priv->alert_data.alert_edge_triggered = 0;
> >
> > Hi George,
> >
> > I think this is not needed anymore, see:
> > 9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert
> >
> > --Jan
>
> Yes.
>
> And also all of this is not needed if named interrupts.
> - interrupt-names
>  ÂÂÂÂ"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
>  ÂÂÂÂother names areÂÂÂ left to individual drivers.
>
> presence of named irq smbus_alert should result in alert handler being
> created for that bus by the core

Thanks for the comments I will send out a v2 for this patch.
> > > +
> > > +ÂÂÂ priv->ara = i2c_setup_smbus_alert(&priv->adapter, 
> > > &priv->alert_data);
> > > +ÂÂÂ if (!priv->ara)
> > > +ÂÂÂÂÂÂÂ return -ENODEV;
> > > +
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > Â static int xlp9xx_i2c_probe(struct platform_device *pdev)
> > > Â {
> > > ÂÂÂÂÂ struct xlp9xx_i2c_dev *priv;
> > > @@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct 
> > > platform_device *pdev)
> > > ÂÂÂÂÂÂÂÂÂ dev_err(&pdev->dev, "invalid irq!\n");
> > > ÂÂÂÂÂÂÂÂÂ return priv->irq;
> > > ÂÂÂÂÂ }
> > > +ÂÂÂ /* SMBAlert irq */
> > > +ÂÂÂ priv->alert_data.irq = platform_get_irq(pdev, 1);
> > > +ÂÂÂ if (priv->alert_data.irq <= 0)
> > > +ÂÂÂÂÂÂÂ priv->alert_data.irq = 0;
> > > ÂÂÂÂÂ xlp9xx_i2c_get_frequency(pdev, priv);
> > > ÂÂÂÂÂ xlp9xx_i2c_init(priv);
> > > @@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct 
> > > platform_device *pdev)
> > > ÂÂÂÂÂ if (err)
> > > ÂÂÂÂÂÂÂÂÂ return err;
> > > +ÂÂÂ err = xlp9xx_i2c_smbus_setup(priv, pdev);
> > > +ÂÂÂ if (err)
> > > +ÂÂÂÂÂÂÂ dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
> > > +
> > > ÂÂÂÂÂ platform_set_drvdata(pdev, priv);
> > > ÂÂÂÂÂ dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
> > > --
> > > 2.1.4