| Message ID | 1557932949-15912-1-git-send-email-fabrice.gasnier@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i2c: i2c-stm32f7: fix the get_irq error cases | expand |
Hi Reviewed-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> Thanks Regards On 5/15/19 5:09 PM, Fabrice Gasnier wrote: > During probe, return the "get_irq" error value instead of -EINVAL which > allows the driver to be deferred probed if needed. > Fix also the case where of_irq_get() returns a negative value. > Note : > On failure of_irq_get() returns 0 or a negative value while > platform_get_irq() returns a negative value. > > Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") > > Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > drivers/i2c/busses/i2c-stm32f7.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > index 4284fc9..14fb105 100644 > --- a/drivers/i2c/busses/i2c-stm32f7.c > +++ b/drivers/i2c/busses/i2c-stm32f7.c > @@ -25,7 +25,6 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_address.h> > -#include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > #include <linux/pinctrl/consumer.h> > @@ -1812,15 +1811,14 @@ static struct i2c_algorithm stm32f7_i2c_algo = { > > static int stm32f7_i2c_probe(struct platform_device *pdev) > { > - struct device_node *np = pdev->dev.of_node; > struct stm32f7_i2c_dev *i2c_dev; > const struct stm32f7_i2c_setup *setup; > struct resource *res; > - u32 irq_error, irq_event, clk_rate, rise_time, fall_time; > + u32 clk_rate, rise_time, fall_time; > struct i2c_adapter *adap; > struct reset_control *rst; > dma_addr_t phy_addr; > - int ret; > + int irq_error, irq_event, ret; > > i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL); > if (!i2c_dev) > @@ -1832,16 +1830,20 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) > return PTR_ERR(i2c_dev->base); > phy_addr = (dma_addr_t)res->start; > > - irq_event = irq_of_parse_and_map(np, 0); > - if (!irq_event) { > - dev_err(&pdev->dev, "IRQ event missing or invalid\n"); > - return -EINVAL; > + irq_event = platform_get_irq(pdev, 0); > + if (irq_event < 0) { > + if (irq_event != -EPROBE_DEFER) > + dev_err(&pdev->dev, "Failed to get IRQ event: %d\n", > + irq_event); > + return irq_event; > } > > - irq_error = irq_of_parse_and_map(np, 1); > - if (!irq_error) { > - dev_err(&pdev->dev, "IRQ error missing or invalid\n"); > - return -EINVAL; > + irq_error = platform_get_irq(pdev, 1); > + if (irq_error < 0) { > + if (irq_error != -EPROBE_DEFER) > + dev_err(&pdev->dev, "Failed to get IRQ error: %d\n", > + irq_error); > + return irq_error; > } > > i2c_dev->clk = devm_clk_get(&pdev->dev, NULL); >
On 15/05/2019 17:09, Fabrice Gasnier wrote: > During probe, return the "get_irq" error value instead of -EINVAL which > allows the driver to be deferred probed if needed. > Fix also the case where of_irq_get() returns a negative value. > Note : > On failure of_irq_get() returns 0 or a negative value while > platform_get_irq() returns a negative value. > > Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") > > Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> > --- > drivers/i2c/busses/i2c-stm32f7.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > index 4284fc9..14fb105 100644 > --- a/drivers/i2c/busses/i2c-stm32f7.c > +++ b/drivers/i2c/busses/i2c-stm32f7.c > @@ -25,7 +25,6 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_address.h> > -#include <linux/of_irq.h> > #include <linux/of_platform.h> > #include <linux/platform_device.h> > #include <linux/pinctrl/consumer.h> > @@ -1812,15 +1811,14 @@ static struct i2c_algorithm stm32f7_i2c_algo = { > > static int stm32f7_i2c_probe(struct platform_device *pdev) > { > - struct device_node *np = pdev->dev.of_node; > struct stm32f7_i2c_dev *i2c_dev; > const struct stm32f7_i2c_setup *setup; > struct resource *res; > - u32 irq_error, irq_event, clk_rate, rise_time, fall_time; > + u32 clk_rate, rise_time, fall_time; > struct i2c_adapter *adap; > struct reset_control *rst; > dma_addr_t phy_addr; > - int ret; > + int irq_error, irq_event, ret; > > i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL); > if (!i2c_dev) > @@ -1832,16 +1830,20 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) > return PTR_ERR(i2c_dev->base); > phy_addr = (dma_addr_t)res->start; > > - irq_event = irq_of_parse_and_map(np, 0); > - if (!irq_event) { > - dev_err(&pdev->dev, "IRQ event missing or invalid\n"); > - return -EINVAL; > + irq_event = platform_get_irq(pdev, 0); > + if (irq_event < 0) { > + if (irq_event != -EPROBE_DEFER) > + dev_err(&pdev->dev, "Failed to get IRQ event: %d\n", > + irq_event); > + return irq_event; > } > > - irq_error = irq_of_parse_and_map(np, 1); > - if (!irq_error) { > - dev_err(&pdev->dev, "IRQ error missing or invalid\n"); > - return -EINVAL; > + irq_error = platform_get_irq(pdev, 1); > + if (irq_error < 0) { > + if (irq_error != -EPROBE_DEFER) > + dev_err(&pdev->dev, "Failed to get IRQ error: %d\n", > + irq_error); > + return irq_error; As far as I understand, if platform_get_irq() returns 0, that is an error as well. https://patchwork.kernel.org/patch/10006651/ https://yarchive.net/comp/linux/zero.html Regards.
On 5/16/19 11:22 AM, Marc Gonzalez wrote: > On 15/05/2019 17:09, Fabrice Gasnier wrote: > >> During probe, return the "get_irq" error value instead of -EINVAL which >> allows the driver to be deferred probed if needed. >> Fix also the case where of_irq_get() returns a negative value. >> Note : >> On failure of_irq_get() returns 0 or a negative value while >> platform_get_irq() returns a negative value. >> >> Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") >> >> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> >> --- >> drivers/i2c/busses/i2c-stm32f7.c | 26 ++++++++++++++------------ >> 1 file changed, 14 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c >> index 4284fc9..14fb105 100644 >> --- a/drivers/i2c/busses/i2c-stm32f7.c >> +++ b/drivers/i2c/busses/i2c-stm32f7.c >> @@ -25,7 +25,6 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/of_address.h> >> -#include <linux/of_irq.h> >> #include <linux/of_platform.h> >> #include <linux/platform_device.h> >> #include <linux/pinctrl/consumer.h> >> @@ -1812,15 +1811,14 @@ static struct i2c_algorithm stm32f7_i2c_algo = { >> >> static int stm32f7_i2c_probe(struct platform_device *pdev) >> { >> - struct device_node *np = pdev->dev.of_node; >> struct stm32f7_i2c_dev *i2c_dev; >> const struct stm32f7_i2c_setup *setup; >> struct resource *res; >> - u32 irq_error, irq_event, clk_rate, rise_time, fall_time; >> + u32 clk_rate, rise_time, fall_time; >> struct i2c_adapter *adap; >> struct reset_control *rst; >> dma_addr_t phy_addr; >> - int ret; >> + int irq_error, irq_event, ret; >> >> i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL); >> if (!i2c_dev) >> @@ -1832,16 +1830,20 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) >> return PTR_ERR(i2c_dev->base); >> phy_addr = (dma_addr_t)res->start; >> >> - irq_event = irq_of_parse_and_map(np, 0); >> - if (!irq_event) { >> - dev_err(&pdev->dev, "IRQ event missing or invalid\n"); >> - return -EINVAL; >> + irq_event = platform_get_irq(pdev, 0); >> + if (irq_event < 0) { >> + if (irq_event != -EPROBE_DEFER) >> + dev_err(&pdev->dev, "Failed to get IRQ event: %d\n", >> + irq_event); >> + return irq_event; >> } >> >> - irq_error = irq_of_parse_and_map(np, 1); >> - if (!irq_error) { >> - dev_err(&pdev->dev, "IRQ error missing or invalid\n"); >> - return -EINVAL; >> + irq_error = platform_get_irq(pdev, 1); >> + if (irq_error < 0) { >> + if (irq_error != -EPROBE_DEFER) >> + dev_err(&pdev->dev, "Failed to get IRQ error: %d\n", >> + irq_error); >> + return irq_error; > > As far as I understand, if platform_get_irq() returns 0, that is an error > as well. > > https://patchwork.kernel.org/patch/10006651/ Hi Marc, Thanks for pointing this out. I'll fix it in v2, based on this, e.g. basically: if (irq <= 0) return irq ? irq : -ENODEV; Best Regards, Fabrice > https://yarchive.net/comp/linux/zero.html > > Regards. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c index 4284fc9..14fb105 100644 --- a/drivers/i2c/busses/i2c-stm32f7.c +++ b/drivers/i2c/busses/i2c-stm32f7.c @@ -25,7 +25,6 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_address.h> -#include <linux/of_irq.h> #include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/pinctrl/consumer.h> @@ -1812,15 +1811,14 @@ static struct i2c_algorithm stm32f7_i2c_algo = { static int stm32f7_i2c_probe(struct platform_device *pdev) { - struct device_node *np = pdev->dev.of_node; struct stm32f7_i2c_dev *i2c_dev; const struct stm32f7_i2c_setup *setup; struct resource *res; - u32 irq_error, irq_event, clk_rate, rise_time, fall_time; + u32 clk_rate, rise_time, fall_time; struct i2c_adapter *adap; struct reset_control *rst; dma_addr_t phy_addr; - int ret; + int irq_error, irq_event, ret; i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL); if (!i2c_dev) @@ -1832,16 +1830,20 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) return PTR_ERR(i2c_dev->base); phy_addr = (dma_addr_t)res->start; - irq_event = irq_of_parse_and_map(np, 0); - if (!irq_event) { - dev_err(&pdev->dev, "IRQ event missing or invalid\n"); - return -EINVAL; + irq_event = platform_get_irq(pdev, 0); + if (irq_event < 0) { + if (irq_event != -EPROBE_DEFER) + dev_err(&pdev->dev, "Failed to get IRQ event: %d\n", + irq_event); + return irq_event; } - irq_error = irq_of_parse_and_map(np, 1); - if (!irq_error) { - dev_err(&pdev->dev, "IRQ error missing or invalid\n"); - return -EINVAL; + irq_error = platform_get_irq(pdev, 1); + if (irq_error < 0) { + if (irq_error != -EPROBE_DEFER) + dev_err(&pdev->dev, "Failed to get IRQ error: %d\n", + irq_error); + return irq_error; } i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
During probe, return the "get_irq" error value instead of -EINVAL which allows the driver to be deferred probed if needed. Fix also the case where of_irq_get() returns a negative value. Note : On failure of_irq_get() returns 0 or a negative value while platform_get_irq() returns a negative value. Fixes: aeb068c57214 ("i2c: i2c-stm32f7: add driver") Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com> --- drivers/i2c/busses/i2c-stm32f7.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)