[PATCH v2 0/3] mmc: support setting card detect interrupt from drivers and use it in meson-gx

[PATCH v2 0/3] mmc: support setting card detect interrupt from drivers and use it in meson-gx

[Heiner Kallweit]

On certain platforms like Amlogic Meson gpiod_to_irq() isn't supported
due to the design of gpio / interrupt controller. Therefore provide an
option for drivers to pass the card detect interrupt number
(retrieved e.g. from device tree) to mmc core.

v2:
- use another mechanism for passing and storing the cd interrupt
- add patch 2

Heiner Kallweit (3):
  mmc: core: support setting card detect interrupt from drivers
  dt-bindings: mmc: meson-gx: support specifying cd interrupt
  mmc: meson-gx: support platform interrupt as card detect interrupt

 .../bindings/mmc/amlogic,meson-gx-mmc.yaml      |  2 +-
 drivers/mmc/core/slot-gpio.c                    | 17 ++++++++++++++++-
 drivers/mmc/host/meson-gx-mmc.c                 |  5 ++++-
 include/linux/mmc/slot-gpio.h                   |  1 +
 4 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.39.1

[PATCH v2 1/3] mmc: core: support setting card detect interrupt from drivers

[Heiner Kallweit]

On certain platforms like Amlogic Meson gpiod_to_irq() isn't supported
due to the design of gpio / interrupt controller. Therefore provide an
option for drivers to pass the card detect interrupt number
(retrieved e.g. from device tree) to mmc core.

Suggested-by refers to the mechanism to pass and store the interrupt.

Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- use another mechanism for passing and storing the cd interrupt
---
 drivers/mmc/core/slot-gpio.c  | 17 ++++++++++++++++-
 include/linux/mmc/slot-gpio.h |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index dd2a4b6ab..2a2d949a9 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -23,6 +23,7 @@ struct mmc_gpio {
 	char *ro_label;
 	char *cd_label;
 	u32 cd_debounce_delay_ms;
+	int cd_irq;
 };
 
 static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
@@ -53,12 +54,24 @@ int mmc_gpio_alloc(struct mmc_host *host)
 	ctx->ro_label = devm_kasprintf(host->parent, GFP_KERNEL, "%s ro", devname);
 	if (!ctx->ro_label)
 		return -ENOMEM;
+	ctx->cd_irq = -EINVAL;
 	host->slot.handler_priv = ctx;
 	host->slot.cd_irq = -EINVAL;
 
 	return 0;
 }
 
+void mmc_gpio_set_cd_irq(struct mmc_host *host, int irq)
+{
+	struct mmc_gpio *ctx = host->slot.handler_priv;
+
+	if (!ctx || irq < 0)
+		return;
+
+	ctx->cd_irq = irq;
+}
+EXPORT_SYMBOL(mmc_gpio_set_cd_irq);
+
 int mmc_gpio_get_ro(struct mmc_host *host)
 {
 	struct mmc_gpio *ctx = host->slot.handler_priv;
@@ -98,7 +111,9 @@ void mmc_gpiod_request_cd_irq(struct mmc_host *host)
 	 * Do not use IRQ if the platform prefers to poll, e.g., because that
 	 * IRQ number is already used by another unit and cannot be shared.
 	 */
-	if (!(host->caps & MMC_CAP_NEEDS_POLL))
+	if (ctx->cd_irq >= 0)
+		irq = ctx->cd_irq;
+	else if (!(host->caps & MMC_CAP_NEEDS_POLL))
 		irq = gpiod_to_irq(ctx->cd_gpio);
 
 	if (irq >= 0) {
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index 4ae2f2908..5d3d15e97 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -15,6 +15,7 @@ struct mmc_host;
 
 int mmc_gpio_get_ro(struct mmc_host *host);
 int mmc_gpio_get_cd(struct mmc_host *host);
+void mmc_gpio_set_cd_irq(struct mmc_host *host, int irq);
 int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
 			 unsigned int idx, bool override_active_level,
 			 unsigned int debounce);
-- 
2.39.1

[PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Heiner Kallweit]

Support passing a second interrupt as card detect interrupt.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
index 46e235bf2..c8c30300d 100644
--- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
+++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
@@ -28,7 +28,7 @@ properties:
     maxItems: 1
 
   interrupts:
-    maxItems: 1
+    maxItems: 2
 
   clocks:
     maxItems: 3
-- 
2.39.1

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Krzysztof Kozlowski]

On 14/02/2023 22:42, Heiner Kallweit wrote:
> Support passing a second interrupt as card detect interrupt.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> index 46e235bf2..c8c30300d 100644
> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> @@ -28,7 +28,7 @@ properties:
>      maxItems: 1
>  
>    interrupts:
> -    maxItems: 1
> +    maxItems: 2

Wait, you now *require* (not support) cd interrupt, so this looks like
ABI break.

Second, you must describe the items.

Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Heiner Kallweit]

On 16.02.2023 10:18, Krzysztof Kozlowski wrote:
> On 14/02/2023 22:42, Heiner Kallweit wrote:
>> Support passing a second interrupt as card detect interrupt.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>> index 46e235bf2..c8c30300d 100644
>> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>> @@ -28,7 +28,7 @@ properties:
>>      maxItems: 1
>>  
>>    interrupts:
>> -    maxItems: 1
>> +    maxItems: 2
> 
> Wait, you now *require* (not support) cd interrupt, so this looks like
> ABI break.
> 
The second interrupt is optional. If not provided MMC core falls back to
cd polling.

> Second, you must describe the items.
> 
OK, this will be added.

> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Krzysztof Kozlowski]

On 16/02/2023 10:29, Heiner Kallweit wrote:
> On 16.02.2023 10:18, Krzysztof Kozlowski wrote:
>> On 14/02/2023 22:42, Heiner Kallweit wrote:
>>> Support passing a second interrupt as card detect interrupt.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>> index 46e235bf2..c8c30300d 100644
>>> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>> @@ -28,7 +28,7 @@ properties:
>>>      maxItems: 1
>>>  
>>>    interrupts:
>>> -    maxItems: 1
>>> +    maxItems: 2
>>
>> Wait, you now *require* (not support) cd interrupt, so this looks like
>> ABI break.
>>
> The second interrupt is optional. If not provided MMC core falls back to
> cd polling.

It is not. Your binding requires it. Did you test it on DTS without the
second interrupt?

> 
>> Second, you must describe the items.
>>
> OK, this will be added.


Best regards,
Krzysztof

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Heiner Kallweit]

On 16.02.2023 10:34, Krzysztof Kozlowski wrote:
> On 16/02/2023 10:29, Heiner Kallweit wrote:
>> On 16.02.2023 10:18, Krzysztof Kozlowski wrote:
>>> On 14/02/2023 22:42, Heiner Kallweit wrote:
>>>> Support passing a second interrupt as card detect interrupt.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>> index 46e235bf2..c8c30300d 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>> @@ -28,7 +28,7 @@ properties:
>>>>      maxItems: 1
>>>>  
>>>>    interrupts:
>>>> -    maxItems: 1
>>>> +    maxItems: 2
>>>
>>> Wait, you now *require* (not support) cd interrupt, so this looks like
>>> ABI break.
>>>
>> The second interrupt is optional. If not provided MMC core falls back to
>> cd polling.
> 
> It is not. Your binding requires it. Did you test it on DTS without the
> second interrupt?
> 
You're right, the binding misses a minItems 1. I'll add that and the
interrupt description.

>>
>>> Second, you must describe the items.
>>>
>> OK, this will be added.
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Heiner Kallweit]

On 16.02.2023 10:41, Heiner Kallweit wrote:
> On 16.02.2023 10:34, Krzysztof Kozlowski wrote:
>> On 16/02/2023 10:29, Heiner Kallweit wrote:
>>> On 16.02.2023 10:18, Krzysztof Kozlowski wrote:
>>>> On 14/02/2023 22:42, Heiner Kallweit wrote:
>>>>> Support passing a second interrupt as card detect interrupt.
>>>>>
>>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>>> index 46e235bf2..c8c30300d 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
>>>>> @@ -28,7 +28,7 @@ properties:
>>>>>      maxItems: 1
>>>>>  
>>>>>    interrupts:
>>>>> -    maxItems: 1
>>>>> +    maxItems: 2
>>>>
>>>> Wait, you now *require* (not support) cd interrupt, so this looks like
>>>> ABI break.
>>>>
>>> The second interrupt is optional. If not provided MMC core falls back to
>>> cd polling.
>>
>> It is not. Your binding requires it. Did you test it on DTS without the
>> second interrupt?
>>
> You're right, the binding misses a minItems 1. I'll add that and the
> interrupt description.
> 
Ulf,
do you require an add-on patch that fixes the binding issue or a
replacement patch?

>>>
>>>> Second, you must describe the items.
>>>>
>>> OK, this will be added.
>>
>>
>> Best regards,
>> Krzysztof
>>
> 

Re: [PATCH v2 2/3] dt-bindings: mmc: meson-gx: support specifying cd interrupt

[Ulf Hansson]

On Thu, 16 Feb 2023 at 10:49, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 16.02.2023 10:41, Heiner Kallweit wrote:
> > On 16.02.2023 10:34, Krzysztof Kozlowski wrote:
> >> On 16/02/2023 10:29, Heiner Kallweit wrote:
> >>> On 16.02.2023 10:18, Krzysztof Kozlowski wrote:
> >>>> On 14/02/2023 22:42, Heiner Kallweit wrote:
> >>>>> Support passing a second interrupt as card detect interrupt.
> >>>>>
> >>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> >>>>> ---
> >>>>>  Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> >>>>> index 46e235bf2..c8c30300d 100644
> >>>>> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-gx-mmc.yaml
> >>>>> @@ -28,7 +28,7 @@ properties:
> >>>>>      maxItems: 1
> >>>>>
> >>>>>    interrupts:
> >>>>> -    maxItems: 1
> >>>>> +    maxItems: 2
> >>>>
> >>>> Wait, you now *require* (not support) cd interrupt, so this looks like
> >>>> ABI break.
> >>>>
> >>> The second interrupt is optional. If not provided MMC core falls back to
> >>> cd polling.
> >>
> >> It is not. Your binding requires it. Did you test it on DTS without the
> >> second interrupt?
> >>
> > You're right, the binding misses a minItems 1. I'll add that and the
> > interrupt description.
> >
> Ulf,
> do you require an add-on patch that fixes the binding issue or a
> replacement patch?

Please send an incremental patch on top, I would like to avoid any
further rebase at this late in the release cycle.

Kind regards
Uffe

[PATCH v2 3/3] mmc: meson-gx: support platform interrupt as card detect interrupt

[Heiner Kallweit]

Use a new mmc core feature and support specifying the card detect
gpio interrupt in device tree.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- adopt the changed way to pass the cd interrupt number
---
 drivers/mmc/host/meson-gx-mmc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index f4b50793e..079c52dbe 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -1166,7 +1166,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct meson_host *host;
 	struct mmc_host *mmc;
-	int ret;
+	int cd_irq, ret;
 
 	mmc = devm_mmc_alloc_host(&pdev->dev, sizeof(struct meson_host));
 	if (!mmc)
@@ -1213,6 +1213,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
 	if (host->irq <= 0)
 		return -EINVAL;
 
+	cd_irq = platform_get_irq_optional(pdev, 1);
+	mmc_gpio_set_cd_irq(mmc, cd_irq);
+
 	host->pinctrl = devm_pinctrl_get(&pdev->dev);
 	if (IS_ERR(host->pinctrl))
 		return PTR_ERR(host->pinctrl);
-- 
2.39.1

Re: [PATCH v2 0/3] mmc: support setting card detect interrupt from drivers and use it in meson-gx

[Ulf Hansson]

On Tue, 14 Feb 2023 at 22:39, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On certain platforms like Amlogic Meson gpiod_to_irq() isn't supported
> due to the design of gpio / interrupt controller. Therefore provide an
> option for drivers to pass the card detect interrupt number
> (retrieved e.g. from device tree) to mmc core.
>
> v2:
> - use another mechanism for passing and storing the cd interrupt
> - add patch 2
>
> Heiner Kallweit (3):
>   mmc: core: support setting card detect interrupt from drivers
>   dt-bindings: mmc: meson-gx: support specifying cd interrupt
>   mmc: meson-gx: support platform interrupt as card detect interrupt
>
>  .../bindings/mmc/amlogic,meson-gx-mmc.yaml      |  2 +-
>  drivers/mmc/core/slot-gpio.c                    | 17 ++++++++++++++++-
>  drivers/mmc/host/meson-gx-mmc.c                 |  5 ++++-
>  include/linux/mmc/slot-gpio.h                   |  1 +
>  4 files changed, 22 insertions(+), 3 deletions(-)
>

Applied for next, thanks!

Kind regards
Uffe

Re: [PATCH v2 0/3] mmc: support setting card detect interrupt from drivers and use it in meson-gx

[Krzysztof Kozlowski]

On 15/02/2023 13:48, Ulf Hansson wrote:
> On Tue, 14 Feb 2023 at 22:39, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On certain platforms like Amlogic Meson gpiod_to_irq() isn't supported
>> due to the design of gpio / interrupt controller. Therefore provide an
>> option for drivers to pass the card detect interrupt number
>> (retrieved e.g. from device tree) to mmc core.
>>
>> v2:
>> - use another mechanism for passing and storing the cd interrupt
>> - add patch 2
>>
>> Heiner Kallweit (3):
>>   mmc: core: support setting card detect interrupt from drivers
>>   dt-bindings: mmc: meson-gx: support specifying cd interrupt
>>   mmc: meson-gx: support platform interrupt as card detect interrupt
>>
>>  .../bindings/mmc/amlogic,meson-gx-mmc.yaml      |  2 +-
>>  drivers/mmc/core/slot-gpio.c                    | 17 ++++++++++++++++-
>>  drivers/mmc/host/meson-gx-mmc.c                 |  5 ++++-
>>  include/linux/mmc/slot-gpio.h                   |  1 +
>>  4 files changed, 22 insertions(+), 3 deletions(-)
>>
> 
> Applied for next, thanks!

A bit too fast. Binding is incorrect - suggests ABI break and makes
interrupts not described enough.

Best regards,
Krzysztof

Re: [PATCH v2 0/3] mmc: support setting card detect interrupt from drivers and use it in meson-gx

[Ulf Hansson]

On Thu, 16 Feb 2023 at 10:18, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 15/02/2023 13:48, Ulf Hansson wrote:
> > On Tue, 14 Feb 2023 at 22:39, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> On certain platforms like Amlogic Meson gpiod_to_irq() isn't supported
> >> due to the design of gpio / interrupt controller. Therefore provide an
> >> option for drivers to pass the card detect interrupt number
> >> (retrieved e.g. from device tree) to mmc core.
> >>
> >> v2:
> >> - use another mechanism for passing and storing the cd interrupt
> >> - add patch 2
> >>
> >> Heiner Kallweit (3):
> >>   mmc: core: support setting card detect interrupt from drivers
> >>   dt-bindings: mmc: meson-gx: support specifying cd interrupt
> >>   mmc: meson-gx: support platform interrupt as card detect interrupt
> >>
> >>  .../bindings/mmc/amlogic,meson-gx-mmc.yaml      |  2 +-
> >>  drivers/mmc/core/slot-gpio.c                    | 17 ++++++++++++++++-
> >>  drivers/mmc/host/meson-gx-mmc.c                 |  5 ++++-
> >>  include/linux/mmc/slot-gpio.h                   |  1 +
> >>  4 files changed, 22 insertions(+), 3 deletions(-)
> >>
> >
> > Applied for next, thanks!
>
> A bit too fast. Binding is incorrect - suggests ABI break and makes
> interrupts not described enough.

Sorry! Thanks for pointing this out. Heiner is fixing this with an
incremental patch on top, please help to review.

Kind regards
Uffe