| Message ID | 20220916120749.2517727-2-benedikt.niedermayr@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gpmc wait-pin additions | expand |
On 16/09/2022 14:07, B. Niedermayr wrote: > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com> > > Newer kernels refuse to probe when using the same wait pin for > different chipselect regions. > But this may be a usecase when connecting for example FPGA or ASIC > modules to the gpmc, which only got one wait pin installed. > > The wait-pin allocation is now tracked by the gpmc driver in order > to be sure that the wait pin has been indeed requested by gpmc. > Therefore the "wait_pin_alloc_mask" has been introduced. > > Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com> > --- > drivers/memory/omap-gpmc.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > index d9bf1c2ac319..ea495e93766b 100644 > --- a/drivers/memory/omap-gpmc.c > +++ b/drivers/memory/omap-gpmc.c > @@ -232,6 +232,7 @@ struct gpmc_device { > int irq; > struct irq_chip irq_chip; > struct gpio_chip gpio_chip; > + unsigned long wait_pin_alloc_mask; > int nirqs; > struct resource *data; > }; > @@ -2221,9 +2222,16 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > GPIO_ACTIVE_HIGH, > GPIOD_IN); > if (IS_ERR(waitpin_desc)) { > - dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); > ret = PTR_ERR(waitpin_desc); > - goto err; > + if (ret == -EBUSY && > + test_bit(wait_pin, &gpmc->wait_pin_alloc_mask)) { > + dev_info(&pdev->dev, "shared wait-pin: %d\n", wait_pin); > + } else { > + dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); > + goto err; > + } > + } else { > + set_bit(wait_pin, &gpmc->wait_pin_alloc_mask); > } And how do you handle shared pin when the original owner unbinds? Best regards, Krzysztof
Hi Krzysztof, On Mon, 2022-09-19 at 11:34 +0200, Krzysztof Kozlowski wrote: > On 16/09/2022 14:07, B. Niedermayr wrote: > > From: Benedikt Niedermayr <benedikt.niedermayr@siemens.com> > > > > Newer kernels refuse to probe when using the same wait pin for > > different chipselect regions. > > But this may be a usecase when connecting for example FPGA or ASIC > > modules to the gpmc, which only got one wait pin installed. > > > > The wait-pin allocation is now tracked by the gpmc driver in order > > to be sure that the wait pin has been indeed requested by gpmc. > > Therefore the "wait_pin_alloc_mask" has been introduced. > > > > Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@siemens.com> > > --- > > drivers/memory/omap-gpmc.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > > index d9bf1c2ac319..ea495e93766b 100644 > > --- a/drivers/memory/omap-gpmc.c > > +++ b/drivers/memory/omap-gpmc.c > > @@ -232,6 +232,7 @@ struct gpmc_device { > > int irq; > > struct irq_chip irq_chip; > > struct gpio_chip gpio_chip; > > + unsigned long wait_pin_alloc_mask; > > int nirqs; > > struct resource *data; > > }; > > @@ -2221,9 +2222,16 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, > > GPIO_ACTIVE_HIGH, > > GPIOD_IN); > > if (IS_ERR(waitpin_desc)) { > > - dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); > > ret = PTR_ERR(waitpin_desc); > > - goto err; > > + if (ret == -EBUSY && > > + test_bit(wait_pin, &gpmc->wait_pin_alloc_mask)) { > > + dev_info(&pdev->dev, "shared wait-pin: %d\n", wait_pin); > > + } else { > > + dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); > > + goto err; > > + } > > + } else { > > + set_bit(wait_pin, &gpmc->wait_pin_alloc_mask); > > } > > And how do you handle shared pin when the original owner unbinds? So first of all this code only keeps track of the wait_pin allocation from within the gpmc. If any other driver/code allocated this pin than the evaluation gpiochip_request_own_desc() would fail since the pin hasn't been requested by the gpmc itself. The testbit() only checks if this pin has been allocated by the gpmc itself. If yes, then the waitpin can be treated as shared wait pin. If no, then another driver allocated the pin before and we return an error. The gpmc must be able to release the wait_pin in the wait_pin_alloc_mask, that's true. The only section where the waitpin_desc is released can be found further down in this function: err_cs: gpiochip_free_own_desc(waitpin_desc); You're right. I must add the relase logic here as well. > > > Best regards, > Krzysztof
On 19/09/2022 14:37, Niedermayr, BENEDIKT wrote: >> >> And how do you handle shared pin when the original owner unbinds? > So first of all this code only keeps track of the wait_pin allocation from within the gpmc. If any other driver/code > allocated this pin than the evaluation gpiochip_request_own_desc() would fail since the pin hasn't been requested by the gpmc itself. > The testbit() only checks if this pin has been allocated by the gpmc itself. If yes, then the waitpin can be treated as shared wait pin. If no, > then another driver allocated the pin before and we return an error. > > The gpmc must be able to release the wait_pin in the wait_pin_alloc_mask, that's true. The only section where the waitpin_desc is released > can be found further down in this function: > > err_cs: > gpiochip_free_own_desc(waitpin_desc); > > You're right. I must add the relase logic here as well. You wrote quite a lot but that was not explanation of how do you handle unbind... Last sentence was enough. Best regards, Krzysztof
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c index d9bf1c2ac319..ea495e93766b 100644 --- a/drivers/memory/omap-gpmc.c +++ b/drivers/memory/omap-gpmc.c @@ -232,6 +232,7 @@ struct gpmc_device { int irq; struct irq_chip irq_chip; struct gpio_chip gpio_chip; + unsigned long wait_pin_alloc_mask; int nirqs; struct resource *data; }; @@ -2221,9 +2222,16 @@ static int gpmc_probe_generic_child(struct platform_device *pdev, GPIO_ACTIVE_HIGH, GPIOD_IN); if (IS_ERR(waitpin_desc)) { - dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); ret = PTR_ERR(waitpin_desc); - goto err; + if (ret == -EBUSY && + test_bit(wait_pin, &gpmc->wait_pin_alloc_mask)) { + dev_info(&pdev->dev, "shared wait-pin: %d\n", wait_pin); + } else { + dev_err(&pdev->dev, "invalid wait-pin: %d\n", wait_pin); + goto err; + } + } else { + set_bit(wait_pin, &gpmc->wait_pin_alloc_mask); } }