Dynamically generate a unique GPIO interrupt name, based on the
device name and the GPIO name. For example:
103: 0 sx1503q 12 Edge sff2-los
104: 0 sx1503q 13 Edge sff3-los
The sffX indicates the SFP the loss of signal GPIO is associated with.
Signed-off-by: Chris Healy <[email protected]>
---
drivers/net/phy/sfp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 73c2969f11a4..9b03c7229320 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -220,6 +220,7 @@ struct sfp {
struct phy_device *mod_phy;
const struct sff_data *type;
u32 max_power_mW;
+ char sfp_irq_name[32];
unsigned int (*get_state)(struct sfp *);
void (*set_state)(struct sfp *, unsigned int);
@@ -2349,12 +2350,15 @@ static int sfp_probe(struct platform_device *pdev)
continue;
}
+ snprintf(sfp->sfp_irq_name, sizeof(sfp->sfp_irq_name),
+ "%s-%s", dev_name(sfp->dev), gpio_of_names[i]);
+
err = devm_request_threaded_irq(sfp->dev, sfp->gpio_irq[i],
NULL, sfp_irq,
IRQF_ONESHOT |
IRQF_TRIGGER_RISING |
IRQF_TRIGGER_FALLING,
- dev_name(sfp->dev), sfp);
+ sfp->sfp_irq_name, sfp);
if (err) {
sfp->gpio_irq[i] = 0;
sfp->need_poll = true;
--
2.21.3
On Mon, Jul 06, 2020 at 12:38:37PM -0700, Chris Healy wrote:
> Dynamically generate a unique GPIO interrupt name, based on the
> device name and the GPIO name. For example:
>
> 103: 0 sx1503q 12 Edge sff2-los
> 104: 0 sx1503q 13 Edge sff3-los
>
> The sffX indicates the SFP the loss of signal GPIO is associated with.
Hi Chris
For netdev, please put inside the [PATCH] part of the subject, which
tree this is for, i.e. net-next.
> Signed-off-by: Chris Healy <[email protected]>
> ---
> drivers/net/phy/sfp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 73c2969f11a4..9b03c7229320 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -220,6 +220,7 @@ struct sfp {
> struct phy_device *mod_phy;
> const struct sff_data *type;
> u32 max_power_mW;
> + char sfp_irq_name[32];
>
> unsigned int (*get_state)(struct sfp *);
> void (*set_state)(struct sfp *, unsigned int);
> @@ -2349,12 +2350,15 @@ static int sfp_probe(struct platform_device *pdev)
> continue;
> }
>
> + snprintf(sfp->sfp_irq_name, sizeof(sfp->sfp_irq_name),
> + "%s-%s", dev_name(sfp->dev), gpio_of_names[i]);
> +
This is perfectly O.K, but you could consider using
devm_kasprintf(). That will allocate as much memory as needed for the
string, and hence avoid truncation issues, which we have seen before
with other interrupt names.
Andrew
On Mon, Jul 06, 2020 at 12:38:37PM -0700, Chris Healy wrote:
> Dynamically generate a unique GPIO interrupt name, based on the
> device name and the GPIO name. For example:
>
> 103: 0 sx1503q 12 Edge sff2-los
> 104: 0 sx1503q 13 Edge sff3-los
>
> The sffX indicates the SFP the loss of signal GPIO is associated with.
>
> Signed-off-by: Chris Healy <[email protected]>
This doesn't work in all cases.
> ---
> drivers/net/phy/sfp.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 73c2969f11a4..9b03c7229320 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -220,6 +220,7 @@ struct sfp {
> struct phy_device *mod_phy;
> const struct sff_data *type;
> u32 max_power_mW;
> + char sfp_irq_name[32];
>
> unsigned int (*get_state)(struct sfp *);
> void (*set_state)(struct sfp *, unsigned int);
> @@ -2349,12 +2350,15 @@ static int sfp_probe(struct platform_device *pdev)
> continue;
> }
>
> + snprintf(sfp->sfp_irq_name, sizeof(sfp->sfp_irq_name),
> + "%s-%s", dev_name(sfp->dev), gpio_of_names[i]);
sfp_irq_name will be overwritten for each GPIO IRQ claimed, which means
all IRQs for a particular cage will end up with the same name.
sfp_irq_name[] therefore needs to be an array of names, one per input.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Jul 6, 2020 at 1:07 PM Andrew Lunn <[email protected]> wrote:
>
> On Mon, Jul 06, 2020 at 12:38:37PM -0700, Chris Healy wrote:
> > Dynamically generate a unique GPIO interrupt name, based on the
> > device name and the GPIO name. For example:
> >
> > 103: 0 sx1503q 12 Edge sff2-los
> > 104: 0 sx1503q 13 Edge sff3-los
> >
> > The sffX indicates the SFP the loss of signal GPIO is associated with.
>
> Hi Chris
>
> For netdev, please put inside the [PATCH] part of the subject, which
> tree this is for, i.e. net-next.
Will do.
>
> > Signed-off-by: Chris Healy <[email protected]>
> > ---
> > drivers/net/phy/sfp.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 73c2969f11a4..9b03c7229320 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -220,6 +220,7 @@ struct sfp {
> > struct phy_device *mod_phy;
> > const struct sff_data *type;
> > u32 max_power_mW;
> > + char sfp_irq_name[32];
> >
> > unsigned int (*get_state)(struct sfp *);
> > void (*set_state)(struct sfp *, unsigned int);
> > @@ -2349,12 +2350,15 @@ static int sfp_probe(struct platform_device *pdev)
> > continue;
> > }
> >
> > + snprintf(sfp->sfp_irq_name, sizeof(sfp->sfp_irq_name),
> > + "%s-%s", dev_name(sfp->dev), gpio_of_names[i]);
> > +
>
> This is perfectly O.K, but you could consider using
> devm_kasprintf(). That will allocate as much memory as needed for the
> string, and hence avoid truncation issues, which we have seen before
> with other interrupt names.
I'll give this a try for the next version of this patch.
>
> Andrew
On Mon, Jul 6, 2020 at 1:42 PM Russell King - ARM Linux admin
<[email protected]> wrote:
>
> On Mon, Jul 06, 2020 at 12:38:37PM -0700, Chris Healy wrote:
> > Dynamically generate a unique GPIO interrupt name, based on the
> > device name and the GPIO name. For example:
> >
> > 103: 0 sx1503q 12 Edge sff2-los
> > 104: 0 sx1503q 13 Edge sff3-los
> >
> > The sffX indicates the SFP the loss of signal GPIO is associated with.
> >
> > Signed-off-by: Chris Healy <[email protected]>
>
> This doesn't work in all cases.
>
> > ---
> > drivers/net/phy/sfp.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > index 73c2969f11a4..9b03c7229320 100644
> > --- a/drivers/net/phy/sfp.c
> > +++ b/drivers/net/phy/sfp.c
> > @@ -220,6 +220,7 @@ struct sfp {
> > struct phy_device *mod_phy;
> > const struct sff_data *type;
> > u32 max_power_mW;
> > + char sfp_irq_name[32];
> >
> > unsigned int (*get_state)(struct sfp *);
> > void (*set_state)(struct sfp *, unsigned int);
> > @@ -2349,12 +2350,15 @@ static int sfp_probe(struct platform_device *pdev)
> > continue;
> > }
> >
> > + snprintf(sfp->sfp_irq_name, sizeof(sfp->sfp_irq_name),
> > + "%s-%s", dev_name(sfp->dev), gpio_of_names[i]);
>
> sfp_irq_name will be overwritten for each GPIO IRQ claimed, which means
> all IRQs for a particular cage will end up with the same name.
> sfp_irq_name[] therefore needs to be an array of names, one per input.
Good point. I'll add the necessary support to deal with this case and
test on my side with a hacked up devicetree file providing some
additional GPIOs and include this in the next version of the patch.
>
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!