* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-16 11:23 Jernej Škrabec
0 siblings, 0 replies; 6+ messages in thread
From: Jernej Škrabec @ 2019-03-16 11:23 UTC (permalink / raw)
To: Vinod Koul
Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
dmaengine, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
Hi!
Dne sobota, 16. marec 2019 ob 12:07:53 CET je Vinod Koul napisal(a):
> On 07-03-19, 17:58, Jernej Skrabec wrote:
> > H6 DMA controller needs additional mbus clock to be enabled.
> >
> > Add a quirk for it and handle it accordingly.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> >
> > drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 0cd13f17fc11..761555080325 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> >
> > u32 dst_burst_lengths;
> > u32 src_addr_widths;
> > u32 dst_addr_widths;
> >
> > + bool mbus_clk;
> >
> > };
> >
> > /*
> >
> > @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> >
> > struct dma_device slave;
> > void __iomem *base;
> > struct clk *clk;
> >
> > + struct clk *clk_mbus;
>
> So rather than have mbus_clk and then a ptr, why not use the ptr and use
> NULL value to check for..?
>
I'm not sure what you mean here. clk_mbus will hold a reference to a clock
retrieved by devm_clk_get() so it has to be "struct clk *".
What I'm missing here?
> > int irq;
> > spinlock_t lock;
> > struct reset_control *rstc;
> >
> > @@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)>
> > return PTR_ERR(sdc->clk);
> >
> > }
> >
> > + if (sdc->cfg->mbus_clk) {
>
> where is the populated? I was expecting this to be set based on DT!
Of course it is based on DT. Check patch 5, where quirks structure attached to
H6 DMA compatible contains ".mbus_clk = true,". "sdc->cfg" points to this
quirk structure.
>
> > + sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
> > + if (IS_ERR(sdc->clk_mbus)) {
> > + dev_err(&pdev->dev, "No mbus clock
specified\n");
> > + return PTR_ERR(sdc->clk_mbus);
> > + }
> > + }
> > +
> >
> > sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
> > if (IS_ERR(sdc->rstc)) {
> >
> > dev_err(&pdev->dev, "No reset controller specified\n");
> >
> > @@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)>
> > goto err_reset_assert;
> >
> > }
> >
> > + if (sdc->cfg->mbus_clk) {
> > + ret = clk_prepare_enable(sdc->clk_mbus);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Couldn't enable mbus
clock\n");
> > + goto err_clk_disable;
> > + }
> > + }
> > +
> >
> > ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt,
0,
> >
> > dev_name(&pdev->dev), sdc);
> >
> > if (ret) {
> >
> > dev_err(&pdev->dev, "Cannot request IRQ\n");
> >
> > - goto err_clk_disable;
> > + goto err_mbus_clk_disable;
> >
> > }
> >
> > ret = dma_async_device_register(&sdc->slave);
> >
> > @@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device
> > *pdev)>
> > dma_async_device_unregister(&sdc->slave);
> >
> > err_irq_disable:
> > sun6i_kill_tasklet(sdc);
> >
> > +err_mbus_clk_disable:
> > + clk_disable_unprepare(sdc->clk_mbus);
> >
> > err_clk_disable:
> > clk_disable_unprepare(sdc->clk);
> >
> > err_reset_assert:
> > @@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device
> > *pdev)>
> > sun6i_kill_tasklet(sdc);
> >
> > + clk_disable_unprepare(sdc->clk_mbus);
> >
> > clk_disable_unprepare(sdc->clk);
> > reset_control_assert(sdc->rstc);
^ permalink raw reply [flat|nested] 6+ messages in thread* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-22 13:03 Maxime Ripard
0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2019-03-22 13:03 UTC (permalink / raw)
To: Vinod Koul
Cc: Jernej Škrabec, wens, robh+dt, mark.rutland, dan.j.williams,
dmaengine, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
On Fri, Mar 22, 2019 at 06:29:40PM +0530, Vinod Koul wrote:
> On 16-03-19, 12:23, Jernej Škrabec wrote:
> > Hi!
> >
> > Dne sobota, 16. marec 2019 ob 12:07:53 CET je Vinod Koul napisal(a):
> > > On 07-03-19, 17:58, Jernej Skrabec wrote:
> > > > H6 DMA controller needs additional mbus clock to be enabled.
> > > >
> > > > Add a quirk for it and handle it accordingly.
> > > >
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > ---
> > > >
> > > > drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> > > > 1 file changed, 22 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > > > index 0cd13f17fc11..761555080325 100644
> > > > --- a/drivers/dma/sun6i-dma.c
> > > > +++ b/drivers/dma/sun6i-dma.c
> > > > @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> > > >
> > > > u32 dst_burst_lengths;
> > > > u32 src_addr_widths;
> > > > u32 dst_addr_widths;
> > > >
> > > > + bool mbus_clk;
> > > >
> > > > };
> > > >
> > > > /*
> > > >
> > > > @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> > > >
> > > > struct dma_device slave;
> > > > void __iomem *base;
> > > > struct clk *clk;
> > > >
> > > > + struct clk *clk_mbus;
> > >
> > > So rather than have mbus_clk and then a ptr, why not use the ptr and use
> > > NULL value to check for..?
> > >
> >
> > I'm not sure what you mean here. clk_mbus will hold a reference to a clock
> > retrieved by devm_clk_get() so it has to be "struct clk *".
> >
> > What I'm missing here?
>
> IIRC there were two variable one clk ptr and one an integer to mark
> presence, you may be able to skip variable and use ptr..
If we're doing that, then we would effectively make it optional. That
DMA engine cannot operate without it.
Maxime
---
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 6+ messages in thread* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-22 12:59 Vinod Koul
0 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2019-03-22 12:59 UTC (permalink / raw)
To: Jernej Škrabec
Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
dmaengine, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
On 16-03-19, 12:23, Jernej Škrabec wrote:
> Hi!
>
> Dne sobota, 16. marec 2019 ob 12:07:53 CET je Vinod Koul napisal(a):
> > On 07-03-19, 17:58, Jernej Skrabec wrote:
> > > H6 DMA controller needs additional mbus clock to be enabled.
> > >
> > > Add a quirk for it and handle it accordingly.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > > drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> > > 1 file changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > > index 0cd13f17fc11..761555080325 100644
> > > --- a/drivers/dma/sun6i-dma.c
> > > +++ b/drivers/dma/sun6i-dma.c
> > > @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> > >
> > > u32 dst_burst_lengths;
> > > u32 src_addr_widths;
> > > u32 dst_addr_widths;
> > >
> > > + bool mbus_clk;
> > >
> > > };
> > >
> > > /*
> > >
> > > @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> > >
> > > struct dma_device slave;
> > > void __iomem *base;
> > > struct clk *clk;
> > >
> > > + struct clk *clk_mbus;
> >
> > So rather than have mbus_clk and then a ptr, why not use the ptr and use
> > NULL value to check for..?
> >
>
> I'm not sure what you mean here. clk_mbus will hold a reference to a clock
> retrieved by devm_clk_get() so it has to be "struct clk *".
>
> What I'm missing here?
IIRC there were two variable one clk ptr and one an integer to mark
presence, you may be able to skip variable and use ptr..
^ permalink raw reply [flat|nested] 6+ messages in thread* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-16 11:07 Vinod Koul
0 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2019-03-16 11:07 UTC (permalink / raw)
To: Jernej Skrabec
Cc: maxime.ripard, wens, robh+dt, mark.rutland, dan.j.williams,
dmaengine, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
On 07-03-19, 17:58, Jernej Skrabec wrote:
> H6 DMA controller needs additional mbus clock to be enabled.
>
> Add a quirk for it and handle it accordingly.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
> drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 0cd13f17fc11..761555080325 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> u32 dst_burst_lengths;
> u32 src_addr_widths;
> u32 dst_addr_widths;
> + bool mbus_clk;
> };
>
> /*
> @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> struct dma_device slave;
> void __iomem *base;
> struct clk *clk;
> + struct clk *clk_mbus;
So rather than have mbus_clk and then a ptr, why not use the ptr and use
NULL value to check for..?
> int irq;
> spinlock_t lock;
> struct reset_control *rstc;
> @@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> return PTR_ERR(sdc->clk);
> }
>
> + if (sdc->cfg->mbus_clk) {
where is the populated? I was expecting this to be set based on DT!
> + sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
> + if (IS_ERR(sdc->clk_mbus)) {
> + dev_err(&pdev->dev, "No mbus clock specified\n");
> + return PTR_ERR(sdc->clk_mbus);
> + }
> + }
> +
> sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
> if (IS_ERR(sdc->rstc)) {
> dev_err(&pdev->dev, "No reset controller specified\n");
> @@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> goto err_reset_assert;
> }
>
> + if (sdc->cfg->mbus_clk) {
> + ret = clk_prepare_enable(sdc->clk_mbus);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't enable mbus clock\n");
> + goto err_clk_disable;
> + }
> + }
> +
> ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
> dev_name(&pdev->dev), sdc);
> if (ret) {
> dev_err(&pdev->dev, "Cannot request IRQ\n");
> - goto err_clk_disable;
> + goto err_mbus_clk_disable;
> }
>
> ret = dma_async_device_register(&sdc->slave);
> @@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> dma_async_device_unregister(&sdc->slave);
> err_irq_disable:
> sun6i_kill_tasklet(sdc);
> +err_mbus_clk_disable:
> + clk_disable_unprepare(sdc->clk_mbus);
> err_clk_disable:
> clk_disable_unprepare(sdc->clk);
> err_reset_assert:
> @@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device *pdev)
>
> sun6i_kill_tasklet(sdc);
>
> + clk_disable_unprepare(sdc->clk_mbus);
> clk_disable_unprepare(sdc->clk);
> reset_control_assert(sdc->rstc);
>
> --
> 2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-11 5:47 Chen-Yu Tsai
0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2019-03-11 5:47 UTC (permalink / raw)
To: Jernej Skrabec
Cc: Maxime Ripard, vkoul, Rob Herring, Mark Rutland, Dan Williams,
dmaengine, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
On Fri, Mar 8, 2019 at 12:58 AM Jernej Skrabec <jernej.skrabec@siol.net> wrote:
>
> H6 DMA controller needs additional mbus clock to be enabled.
>
> Add a quirk for it and handle it accordingly.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
> drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 0cd13f17fc11..761555080325 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -129,6 +129,7 @@ struct sun6i_dma_config {
> u32 dst_burst_lengths;
> u32 src_addr_widths;
> u32 dst_addr_widths;
> + bool mbus_clk;
Nit: has_mbus_clk. Be explicit. Leave nothing to assumptions.
> };
>
> /*
> @@ -182,6 +183,7 @@ struct sun6i_dma_dev {
> struct dma_device slave;
> void __iomem *base;
> struct clk *clk;
> + struct clk *clk_mbus;
> int irq;
> spinlock_t lock;
> struct reset_control *rstc;
> @@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> return PTR_ERR(sdc->clk);
> }
>
> + if (sdc->cfg->mbus_clk) {
> + sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
> + if (IS_ERR(sdc->clk_mbus)) {
> + dev_err(&pdev->dev, "No mbus clock specified\n");
> + return PTR_ERR(sdc->clk_mbus);
> + }
> + }
> +
> sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
> if (IS_ERR(sdc->rstc)) {
> dev_err(&pdev->dev, "No reset controller specified\n");
> @@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> goto err_reset_assert;
> }
>
> + if (sdc->cfg->mbus_clk) {
> + ret = clk_prepare_enable(sdc->clk_mbus);
The clk API checks for NULL pointer, so you could just drop the
conditional here.
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't enable mbus clock\n");
> + goto err_clk_disable;
> + }
> + }
> +
> ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
> dev_name(&pdev->dev), sdc);
> if (ret) {
> dev_err(&pdev->dev, "Cannot request IRQ\n");
> - goto err_clk_disable;
> + goto err_mbus_clk_disable;
> }
>
> ret = dma_async_device_register(&sdc->slave);
> @@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> dma_async_device_unregister(&sdc->slave);
> err_irq_disable:
> sun6i_kill_tasklet(sdc);
> +err_mbus_clk_disable:
> + clk_disable_unprepare(sdc->clk_mbus);
Since you aren't using it here either.
ChenYu
> err_clk_disable:
> clk_disable_unprepare(sdc->clk);
> err_reset_assert:
> @@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device *pdev)
>
> sun6i_kill_tasklet(sdc);
>
> + clk_disable_unprepare(sdc->clk_mbus);
> clk_disable_unprepare(sdc->clk);
> reset_control_assert(sdc->rstc);
>
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock
@ 2019-03-07 16:58 Jernej Škrabec
0 siblings, 0 replies; 6+ messages in thread
From: Jernej Škrabec @ 2019-03-07 16:58 UTC (permalink / raw)
To: maxime.ripard, wens
Cc: vkoul, robh+dt, mark.rutland, dan.j.williams, dmaengine,
devicetree, linux-arm-kernel, linux-kernel, linux-sunxi,
jernej.skrabec
H6 DMA controller needs additional mbus clock to be enabled.
Add a quirk for it and handle it accordingly.
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
drivers/dma/sun6i-dma.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 0cd13f17fc11..761555080325 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -129,6 +129,7 @@ struct sun6i_dma_config {
u32 dst_burst_lengths;
u32 src_addr_widths;
u32 dst_addr_widths;
+ bool mbus_clk;
};
/*
@@ -182,6 +183,7 @@ struct sun6i_dma_dev {
struct dma_device slave;
void __iomem *base;
struct clk *clk;
+ struct clk *clk_mbus;
int irq;
spinlock_t lock;
struct reset_control *rstc;
@@ -1208,6 +1210,14 @@ static int sun6i_dma_probe(struct platform_device *pdev)
return PTR_ERR(sdc->clk);
}
+ if (sdc->cfg->mbus_clk) {
+ sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
+ if (IS_ERR(sdc->clk_mbus)) {
+ dev_err(&pdev->dev, "No mbus clock specified\n");
+ return PTR_ERR(sdc->clk_mbus);
+ }
+ }
+
sdc->rstc = devm_reset_control_get(&pdev->dev, NULL);
if (IS_ERR(sdc->rstc)) {
dev_err(&pdev->dev, "No reset controller specified\n");
@@ -1312,11 +1322,19 @@ static int sun6i_dma_probe(struct platform_device *pdev)
goto err_reset_assert;
}
+ if (sdc->cfg->mbus_clk) {
+ ret = clk_prepare_enable(sdc->clk_mbus);
+ if (ret) {
+ dev_err(&pdev->dev, "Couldn't enable mbus clock\n");
+ goto err_clk_disable;
+ }
+ }
+
ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
dev_name(&pdev->dev), sdc);
if (ret) {
dev_err(&pdev->dev, "Cannot request IRQ\n");
- goto err_clk_disable;
+ goto err_mbus_clk_disable;
}
ret = dma_async_device_register(&sdc->slave);
@@ -1341,6 +1359,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
dma_async_device_unregister(&sdc->slave);
err_irq_disable:
sun6i_kill_tasklet(sdc);
+err_mbus_clk_disable:
+ clk_disable_unprepare(sdc->clk_mbus);
err_clk_disable:
clk_disable_unprepare(sdc->clk);
err_reset_assert:
@@ -1359,6 +1379,7 @@ static int sun6i_dma_remove(struct platform_device *pdev)
sun6i_kill_tasklet(sdc);
+ clk_disable_unprepare(sdc->clk_mbus);
clk_disable_unprepare(sdc->clk);
reset_control_assert(sdc->rstc);
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-22 13:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-16 11:23 [2/6] dmaengine: sun6i: Add a quirk for additional mbus clock Jernej Škrabec
-- strict thread matches above, loose matches on Subject: below --
2019-03-22 13:03 Maxime Ripard
2019-03-22 12:59 Vinod Koul
2019-03-16 11:07 Vinod Koul
2019-03-11 5:47 Chen-Yu Tsai
2019-03-07 16:58 Jernej Škrabec
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox