2018-03-06 14:48:02

by George Cherian

[permalink] [raw]
Subject: [PATCH v2] i2c: xlp9xx: Add support for SMBAlert

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 <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
drivers/i2c/busses/i2c-xlp9xx.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index eb8913e..d5cadb6 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,19 @@ 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->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 +483,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 +513,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);

--
1.8.3.1



2018-03-17 21:08:52

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v2] i2c: xlp9xx: Add support for SMBAlert

On Tue, Mar 06, 2018 at 06:46:34AM -0800, 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 <[email protected]>
> Signed-off-by: George Cherian <[email protected]>

Are the previous reviewers happy now?

> ---
> drivers/i2c/busses/i2c-xlp9xx.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index eb8913e..d5cadb6 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,19 @@ 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->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 +483,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)

'< 0' should do, or?

> + priv->alert_data.irq = 0;
>
> xlp9xx_i2c_get_frequency(pdev, priv);
> xlp9xx_i2c_init(priv);
> @@ -493,6 +513,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);
> +

Do you really want to print this info every time SMBus alert is not
used? Is it common on this platform

> platform_set_drvdata(pdev, priv);
> dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
>
> --
> 1.8.3.1
>


Attachments:
(No filename) (2.60 kB)
signature.asc (849.00 B)
Download all attachments

2018-03-19 11:35:32

by Jan Glauber

[permalink] [raw]
Subject: Re: [PATCH v2] i2c: xlp9xx: Add support for SMBAlert

On Sat, Mar 17, 2018 at 10:03:00PM +0100, Wolfram Sang wrote:
> On Tue, Mar 06, 2018 at 06:46:34AM -0800, 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 <[email protected]>
> > Signed-off-by: George Cherian <[email protected]>
>
> Are the previous reviewers happy now?

Reviewed-by: Jan Glauber <[email protected]>

> > ---
> > drivers/i2c/busses/i2c-xlp9xx.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> > index eb8913e..d5cadb6 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,19 @@ 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->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 +483,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)
>
> '< 0' should do, or?
>
> > + priv->alert_data.irq = 0;
> >
> > xlp9xx_i2c_get_frequency(pdev, priv);
> > xlp9xx_i2c_init(priv);
> > @@ -493,6 +513,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);
> > +
>
> Do you really want to print this info every time SMBus alert is not
> used? Is it common on this platform

We do the same thing in the ThunderX driver and thought it is useful to
have the info print.

--Jan