| Message ID | 20211227094526.698714-19-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2,01/23] counter: Use container_of instead of drvdata to track counter_device | expand |
On Mon, 27 Dec 2021 10:45:21 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > This fixes device lifetime issues where it was possible to free a live > struct device. > > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Assuming you move the 104-quad-8 stuff to the earlier patch. > --- > drivers/counter/104-quad-8.c | 7 ++++--- > drivers/counter/ftm-quaddec.c | 27 +++++++++++++-------------- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c > index 4315b14f239e..680c7ba943a4 100644 > --- a/drivers/counter/104-quad-8.c > +++ b/drivers/counter/104-quad-8.c Block in wrong patch. I'll assume you'll fix that and ignore it beyond saying this change is fine so keep my RB on the earlier patch once you've added this block. > @@ -1084,7 +1084,8 @@ static struct counter_count quad8_counts[] = { > > static irqreturn_t quad8_irq_handler(int irq, void *private) > { > - struct quad8 *const priv = private; > + struct counter_device *counter = private; > + struct quad8 *const priv = counter_priv(counter); > const unsigned long base = priv->base; > unsigned long irq_status; > unsigned long channel; > @@ -1115,7 +1116,7 @@ static irqreturn_t quad8_irq_handler(int irq, void *private) > continue; > } > > - counter_push_event(&priv->counter, event, channel); > + counter_push_event(counter, event, channel); > } > > /* Clear pending interrupts on device */ > @@ -1192,7 +1193,7 @@ static int quad8_probe(struct device *dev, unsigned int id) > outb(QUAD8_CHAN_OP_ENABLE_INTERRUPT_FUNC, base[id] + QUAD8_REG_CHAN_OP); > > err = devm_request_irq(dev, irq[id], quad8_irq_handler, IRQF_SHARED, > - counter->name, priv); > + counter->name, counter); > if (err) > return err; > > diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c > index f5d92df6a611..c636183b1337 100644 > --- a/drivers/counter/ftm-quaddec.c > +++ b/drivers/counter/ftm-quaddec.c > @@ -26,7 +26,6 @@ > }) > > struct ftm_quaddec { > - struct counter_device counter; > struct platform_device *pdev; > void __iomem *ftm_base; > bool big_endian; > @@ -259,15 +258,16 @@ static struct counter_count ftm_quaddec_counts = { > > static int ftm_quaddec_probe(struct platform_device *pdev) > { > + struct counter_device *counter; > struct ftm_quaddec *ftm; > - Try to avoid white space changes in a patch like these even if they seem trivial. The 'noise' adds up. > struct device_node *node = pdev->dev.of_node; > struct resource *io; > int ret; > > - ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); > - if (!ftm) > + counter = devm_counter_alloc(&pdev->dev, sizeof(*ftm)); > + if (!counter) > return -ENOMEM; > + ftm = counter_priv(counter); > > io = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!io) { > @@ -283,14 +283,13 @@ static int ftm_quaddec_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "Failed to map memory region\n"); > return -EINVAL; > } > - ftm->counter.name = dev_name(&pdev->dev); > - ftm->counter.parent = &pdev->dev; > - ftm->counter.ops = &ftm_quaddec_cnt_ops; > - ftm->counter.counts = &ftm_quaddec_counts; > - ftm->counter.num_counts = 1; > - ftm->counter.signals = ftm_quaddec_signals; > - ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); > - ftm->counter.priv = ftm; > + counter->name = dev_name(&pdev->dev); > + counter->parent = &pdev->dev; > + counter->ops = &ftm_quaddec_cnt_ops; > + counter->counts = &ftm_quaddec_counts; > + counter->num_counts = 1; > + counter->signals = ftm_quaddec_signals; > + counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals); > > mutex_init(&ftm->ftm_quaddec_mutex); > > @@ -300,9 +299,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev) > if (ret) > return ret; > > - ret = devm_counter_register(&pdev->dev, &ftm->counter); > + ret = devm_counter_add(&pdev->dev, counter); > if (ret) > - return ret; > + return dev_err_probe(&pdev->dev, ret, "Failed to add counter\n"); > > return 0; > }
On Mon, Dec 27, 2021 at 10:45:21AM +0100, Uwe Kleine-König wrote: > This fixes device lifetime issues where it was possible to free a live > struct device. > > Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > drivers/counter/104-quad-8.c | 7 ++++--- > drivers/counter/ftm-quaddec.c | 27 +++++++++++++-------------- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c > index 4315b14f239e..680c7ba943a4 100644 > --- a/drivers/counter/104-quad-8.c > +++ b/drivers/counter/104-quad-8.c > @@ -1084,7 +1084,8 @@ static struct counter_count quad8_counts[] = { > > static irqreturn_t quad8_irq_handler(int irq, void *private) > { > - struct quad8 *const priv = private; > + struct counter_device *counter = private; > + struct quad8 *const priv = counter_priv(counter); > const unsigned long base = priv->base; > unsigned long irq_status; > unsigned long channel; > @@ -1115,7 +1116,7 @@ static irqreturn_t quad8_irq_handler(int irq, void *private) > continue; > } > > - counter_push_event(&priv->counter, event, channel); > + counter_push_event(counter, event, channel); > } > > /* Clear pending interrupts on device */ > @@ -1192,7 +1193,7 @@ static int quad8_probe(struct device *dev, unsigned int id) > outb(QUAD8_CHAN_OP_ENABLE_INTERRUPT_FUNC, base[id] + QUAD8_REG_CHAN_OP); > > err = devm_request_irq(dev, irq[id], quad8_irq_handler, IRQF_SHARED, > - counter->name, priv); > + counter->name, counter); > if (err) > return err; > > diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c > index f5d92df6a611..c636183b1337 100644 > --- a/drivers/counter/ftm-quaddec.c > +++ b/drivers/counter/ftm-quaddec.c > @@ -26,7 +26,6 @@ > }) > > struct ftm_quaddec { > - struct counter_device counter; > struct platform_device *pdev; > void __iomem *ftm_base; > bool big_endian; > @@ -259,15 +258,16 @@ static struct counter_count ftm_quaddec_counts = { > > static int ftm_quaddec_probe(struct platform_device *pdev) > { > + struct counter_device *counter; > struct ftm_quaddec *ftm; > - > struct device_node *node = pdev->dev.of_node; > struct resource *io; > int ret; > > - ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); > - if (!ftm) > + counter = devm_counter_alloc(&pdev->dev, sizeof(*ftm)); > + if (!counter) > return -ENOMEM; > + ftm = counter_priv(counter); > > io = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!io) { > @@ -283,14 +283,13 @@ static int ftm_quaddec_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "Failed to map memory region\n"); > return -EINVAL; > } > - ftm->counter.name = dev_name(&pdev->dev); > - ftm->counter.parent = &pdev->dev; > - ftm->counter.ops = &ftm_quaddec_cnt_ops; > - ftm->counter.counts = &ftm_quaddec_counts; > - ftm->counter.num_counts = 1; > - ftm->counter.signals = ftm_quaddec_signals; > - ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); > - ftm->counter.priv = ftm; > + counter->name = dev_name(&pdev->dev); > + counter->parent = &pdev->dev; > + counter->ops = &ftm_quaddec_cnt_ops; > + counter->counts = &ftm_quaddec_counts; > + counter->num_counts = 1; > + counter->signals = ftm_quaddec_signals; > + counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals); > > mutex_init(&ftm->ftm_quaddec_mutex); > > @@ -300,9 +299,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev) > if (ret) > return ret; > > - ret = devm_counter_register(&pdev->dev, &ftm->counter); > + ret = devm_counter_add(&pdev->dev, counter); > if (ret) > - return ret; > + return dev_err_probe(&pdev->dev, ret, "Failed to add counter\n"); > > return 0; > } > -- > 2.33.0 >
diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c index 4315b14f239e..680c7ba943a4 100644 --- a/drivers/counter/104-quad-8.c +++ b/drivers/counter/104-quad-8.c @@ -1084,7 +1084,8 @@ static struct counter_count quad8_counts[] = { static irqreturn_t quad8_irq_handler(int irq, void *private) { - struct quad8 *const priv = private; + struct counter_device *counter = private; + struct quad8 *const priv = counter_priv(counter); const unsigned long base = priv->base; unsigned long irq_status; unsigned long channel; @@ -1115,7 +1116,7 @@ static irqreturn_t quad8_irq_handler(int irq, void *private) continue; } - counter_push_event(&priv->counter, event, channel); + counter_push_event(counter, event, channel); } /* Clear pending interrupts on device */ @@ -1192,7 +1193,7 @@ static int quad8_probe(struct device *dev, unsigned int id) outb(QUAD8_CHAN_OP_ENABLE_INTERRUPT_FUNC, base[id] + QUAD8_REG_CHAN_OP); err = devm_request_irq(dev, irq[id], quad8_irq_handler, IRQF_SHARED, - counter->name, priv); + counter->name, counter); if (err) return err; diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c index f5d92df6a611..c636183b1337 100644 --- a/drivers/counter/ftm-quaddec.c +++ b/drivers/counter/ftm-quaddec.c @@ -26,7 +26,6 @@ }) struct ftm_quaddec { - struct counter_device counter; struct platform_device *pdev; void __iomem *ftm_base; bool big_endian; @@ -259,15 +258,16 @@ static struct counter_count ftm_quaddec_counts = { static int ftm_quaddec_probe(struct platform_device *pdev) { + struct counter_device *counter; struct ftm_quaddec *ftm; - struct device_node *node = pdev->dev.of_node; struct resource *io; int ret; - ftm = devm_kzalloc(&pdev->dev, sizeof(*ftm), GFP_KERNEL); - if (!ftm) + counter = devm_counter_alloc(&pdev->dev, sizeof(*ftm)); + if (!counter) return -ENOMEM; + ftm = counter_priv(counter); io = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!io) { @@ -283,14 +283,13 @@ static int ftm_quaddec_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to map memory region\n"); return -EINVAL; } - ftm->counter.name = dev_name(&pdev->dev); - ftm->counter.parent = &pdev->dev; - ftm->counter.ops = &ftm_quaddec_cnt_ops; - ftm->counter.counts = &ftm_quaddec_counts; - ftm->counter.num_counts = 1; - ftm->counter.signals = ftm_quaddec_signals; - ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals); - ftm->counter.priv = ftm; + counter->name = dev_name(&pdev->dev); + counter->parent = &pdev->dev; + counter->ops = &ftm_quaddec_cnt_ops; + counter->counts = &ftm_quaddec_counts; + counter->num_counts = 1; + counter->signals = ftm_quaddec_signals; + counter->num_signals = ARRAY_SIZE(ftm_quaddec_signals); mutex_init(&ftm->ftm_quaddec_mutex); @@ -300,9 +299,9 @@ static int ftm_quaddec_probe(struct platform_device *pdev) if (ret) return ret; - ret = devm_counter_register(&pdev->dev, &ftm->counter); + ret = devm_counter_add(&pdev->dev, counter); if (ret) - return ret; + return dev_err_probe(&pdev->dev, ret, "Failed to add counter\n"); return 0; }
This fixes device lifetime issues where it was possible to free a live struct device. Fixes: a3b9a99980d9 ("counter: add FlexTimer Module Quadrature decoder counter driver") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/counter/104-quad-8.c | 7 ++++--- drivers/counter/ftm-quaddec.c | 27 +++++++++++++-------------- 2 files changed, 17 insertions(+), 17 deletions(-)