| Message ID | 20211117231614.758362-1-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] pinctrl: cherryview: Don't use pin/offset 0 to mark an interrupt-line as unused | expand |
Hi Hans, Few minor comments below. On Thu, Nov 18, 2021 at 12:16:12AM +0100, Hans de Goede wrote: > offset/pin 0 is a perfectly valid offset, so stop using it to have Offset/pin (with capital O). > the special meaning of interrupt-line not used in the intr_lines. interrupt line > Instead introduce a new special INTR_LINE_UNUSED value which is never > a valid offset and use that to indicate unused interrupt-lines. interrupt lines > Cc: Yauhen Kharuzhy <jekhor@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/pinctrl/intel/pinctrl-cherryview.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c > index 980099028cf8..a46f9e5a4748 100644 > --- a/drivers/pinctrl/intel/pinctrl-cherryview.c > +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c > @@ -73,6 +73,8 @@ struct intel_pad_context { > u32 padctrl1; > }; > > +#define INTR_LINE_UNUSED U32_MAX > + > /** > * struct intel_community_context - community context for Cherryview > * @intr_lines: Mapping between 16 HW interrupt wires and GPIO offset (in GPIO number space) > @@ -812,7 +814,7 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev, > /* Reset the interrupt mapping */ > for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) { > if (cctx->intr_lines[i] == offset) { > - cctx->intr_lines[i] = 0; > + cctx->intr_lines[i] = INTR_LINE_UNUSED; > break; > } > } > @@ -1319,7 +1321,7 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d) > else > handler = handle_edge_irq; > > - if (!cctx->intr_lines[intsel]) { > + if (cctx->intr_lines[intsel] == INTR_LINE_UNUSED) { > irq_set_handler_locked(d, handler); > cctx->intr_lines[intsel] = pin; > } > @@ -1412,6 +1414,12 @@ static void chv_gpio_irq_handler(struct irq_desc *desc) > unsigned int offset; > > offset = cctx->intr_lines[intr_line]; > + if (offset == INTR_LINE_UNUSED) { > + dev_err(pctrl->dev, "Interrupt on unused interrupt line %u\n", Let's be consistent with the logging so no capital letter here => "interrupt on .." > + intr_line); > + continue; > + } > + > generic_handle_domain_irq(gc->irq.domain, offset); > } > > @@ -1620,9 +1628,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) > struct intel_community *community; > struct device *dev = &pdev->dev; > struct acpi_device *adev = ACPI_COMPANION(dev); > + struct intel_community_context *cctx; > struct intel_pinctrl *pctrl; > acpi_status status; > - int ret, irq; > + int i, ret, irq; > > soc_data = intel_pinctrl_get_soc_data(pdev); > if (IS_ERR(soc_data)) > @@ -1663,6 +1672,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) > if (!pctrl->context.communities) > return -ENOMEM; > > + cctx = &pctrl->context.communities[0]; > + for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) > + cctx->intr_lines[i] = INTR_LINE_UNUSED; > + > irq = platform_get_irq(pdev, 0); > if (irq < 0) > return irq; > -- > 2.31.1
Hi Mika, On 11/18/21 11:13, Mika Westerberg wrote: > Hi Hans, > > Few minor comments below. Thank you for the review. I'll send a new version addressing all your remarks. Regards, Hans > > On Thu, Nov 18, 2021 at 12:16:12AM +0100, Hans de Goede wrote: >> offset/pin 0 is a perfectly valid offset, so stop using it to have > > Offset/pin (with capital O). > >> the special meaning of interrupt-line not used in the intr_lines. > > interrupt line > >> Instead introduce a new special INTR_LINE_UNUSED value which is never >> a valid offset and use that to indicate unused interrupt-lines. > > interrupt lines > >> Cc: Yauhen Kharuzhy <jekhor@gmail.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/pinctrl/intel/pinctrl-cherryview.c | 19 ++++++++++++++++--- >> 1 file changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c >> index 980099028cf8..a46f9e5a4748 100644 >> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c >> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c >> @@ -73,6 +73,8 @@ struct intel_pad_context { >> u32 padctrl1; >> }; >> >> +#define INTR_LINE_UNUSED U32_MAX >> + >> /** >> * struct intel_community_context - community context for Cherryview >> * @intr_lines: Mapping between 16 HW interrupt wires and GPIO offset (in GPIO number space) >> @@ -812,7 +814,7 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev, >> /* Reset the interrupt mapping */ >> for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) { >> if (cctx->intr_lines[i] == offset) { >> - cctx->intr_lines[i] = 0; >> + cctx->intr_lines[i] = INTR_LINE_UNUSED; >> break; >> } >> } >> @@ -1319,7 +1321,7 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d) >> else >> handler = handle_edge_irq; >> >> - if (!cctx->intr_lines[intsel]) { >> + if (cctx->intr_lines[intsel] == INTR_LINE_UNUSED) { >> irq_set_handler_locked(d, handler); >> cctx->intr_lines[intsel] = pin; >> } >> @@ -1412,6 +1414,12 @@ static void chv_gpio_irq_handler(struct irq_desc *desc) >> unsigned int offset; >> >> offset = cctx->intr_lines[intr_line]; >> + if (offset == INTR_LINE_UNUSED) { >> + dev_err(pctrl->dev, "Interrupt on unused interrupt line %u\n", > > Let's be consistent with the logging so no capital letter here => > "interrupt on .." > >> + intr_line); >> + continue; >> + } >> + >> generic_handle_domain_irq(gc->irq.domain, offset); >> } >> >> @@ -1620,9 +1628,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) >> struct intel_community *community; >> struct device *dev = &pdev->dev; >> struct acpi_device *adev = ACPI_COMPANION(dev); >> + struct intel_community_context *cctx; >> struct intel_pinctrl *pctrl; >> acpi_status status; >> - int ret, irq; >> + int i, ret, irq; >> >> soc_data = intel_pinctrl_get_soc_data(pdev); >> if (IS_ERR(soc_data)) >> @@ -1663,6 +1672,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) >> if (!pctrl->context.communities) >> return -ENOMEM; >> >> + cctx = &pctrl->context.communities[0]; >> + for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) >> + cctx->intr_lines[i] = INTR_LINE_UNUSED; >> + >> irq = platform_get_irq(pdev, 0); >> if (irq < 0) >> return irq; >> -- >> 2.31.1 >
diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index 980099028cf8..a46f9e5a4748 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -73,6 +73,8 @@ struct intel_pad_context { u32 padctrl1; }; +#define INTR_LINE_UNUSED U32_MAX + /** * struct intel_community_context - community context for Cherryview * @intr_lines: Mapping between 16 HW interrupt wires and GPIO offset (in GPIO number space) @@ -812,7 +814,7 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev, /* Reset the interrupt mapping */ for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) { if (cctx->intr_lines[i] == offset) { - cctx->intr_lines[i] = 0; + cctx->intr_lines[i] = INTR_LINE_UNUSED; break; } } @@ -1319,7 +1321,7 @@ static unsigned chv_gpio_irq_startup(struct irq_data *d) else handler = handle_edge_irq; - if (!cctx->intr_lines[intsel]) { + if (cctx->intr_lines[intsel] == INTR_LINE_UNUSED) { irq_set_handler_locked(d, handler); cctx->intr_lines[intsel] = pin; } @@ -1412,6 +1414,12 @@ static void chv_gpio_irq_handler(struct irq_desc *desc) unsigned int offset; offset = cctx->intr_lines[intr_line]; + if (offset == INTR_LINE_UNUSED) { + dev_err(pctrl->dev, "Interrupt on unused interrupt line %u\n", + intr_line); + continue; + } + generic_handle_domain_irq(gc->irq.domain, offset); } @@ -1620,9 +1628,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) struct intel_community *community; struct device *dev = &pdev->dev; struct acpi_device *adev = ACPI_COMPANION(dev); + struct intel_community_context *cctx; struct intel_pinctrl *pctrl; acpi_status status; - int ret, irq; + int i, ret, irq; soc_data = intel_pinctrl_get_soc_data(pdev); if (IS_ERR(soc_data)) @@ -1663,6 +1672,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) if (!pctrl->context.communities) return -ENOMEM; + cctx = &pctrl->context.communities[0]; + for (i = 0; i < ARRAY_SIZE(cctx->intr_lines); i++) + cctx->intr_lines[i] = INTR_LINE_UNUSED; + irq = platform_get_irq(pdev, 0); if (irq < 0) return irq;
offset/pin 0 is a perfectly valid offset, so stop using it to have the special meaning of interrupt-line not used in the intr_lines. Instead introduce a new special INTR_LINE_UNUSED value which is never a valid offset and use that to indicate unused interrupt-lines. Cc: Yauhen Kharuzhy <jekhor@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/pinctrl/intel/pinctrl-cherryview.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)