| Message ID | 20241102093140.2625230-3-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v5,1/5] dma-engine: sun4i: Add a quirk to support different chips | expand |
On Sat, 2 Nov 2024 10:31:41 +0100 "Csókás, Bence" <csokas.bence@prolan.hu> wrote: Hi, > From: Mesih Kilinc <mesihkilinc@gmail.com> > > Allwinner suniv F1C100s has a reset bit for DMA in CCU. Sun4i do not > has this bit but in order to support suniv we need to add it. So add > support for reset bit. > > Signed-off-by: Mesih Kilinc <mesihkilinc@gmail.com> > [ csokas.bence: Rebased and addressed comments ] > Signed-off-by: Csókás, Bence <csokas.bence@prolan.hu> > --- > > Notes: > Changes in v2: > * Call reset_control_deassert() unconditionally, as it supports optional resets > * Use dev_err_probe() > * Whitespace > Changes in v3: > * More dev_err_probe() fixes > Changes in v4: > * Use return value of dev_err_probe() > Changes in v5: > * More whitespace > > drivers/dma/sun4i-dma.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c > index b2c1e4b9f696..9d1e3c51342d 100644 > --- a/drivers/dma/sun4i-dma.c > +++ b/drivers/dma/sun4i-dma.c > @@ -15,6 +15,7 @@ > #include <linux/of_dma.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > +#include <linux/reset.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > > @@ -159,6 +160,7 @@ struct sun4i_dma_config { > u8 ddma_drq_sdram; > > u8 max_burst; > + bool has_reset; > }; > > struct sun4i_dma_pchan { > @@ -208,6 +210,7 @@ struct sun4i_dma_dev { > int irq; > spinlock_t lock; > const struct sun4i_dma_config *cfg; > + struct reset_control *rst; > }; > > static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) > @@ -1215,6 +1218,13 @@ static int sun4i_dma_probe(struct platform_device *pdev) > return PTR_ERR(priv->clk); > } > > + if (priv->cfg->has_reset) { > + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); Can't we use devm_reset_control_get_optional_exclusive(), and then save this whole has_reset bit? > + if (IS_ERR(priv->rst)) > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->rst), > + "Failed to get reset control\n"); > + } > + > platform_set_drvdata(pdev, priv); > spin_lock_init(&priv->lock); > > @@ -1287,6 +1297,14 @@ static int sun4i_dma_probe(struct platform_device *pdev) > return ret; > } > > + /* Deassert the reset control */ > + ret = reset_control_deassert(priv->rst); > + if (ret) { > + dev_err_probe(&pdev->dev, ret, > + "Failed to deassert the reset control\n"); > + goto err_clk_disable; > + } > + > /* > * Make sure the IRQs are all disabled and accounted for. The bootloader > * likes to leave these dirty > @@ -1355,6 +1373,7 @@ static struct sun4i_dma_config sun4i_a10_dma_cfg = { > .ddma_drq_sdram = SUN4I_DDMA_DRQ_TYPE_SDRAM, > > .max_burst = SUN4I_MAX_BURST, > + .has_reset = false, > }; > > static const struct of_device_id sun4i_dma_match[] = {
Hi! On 2024. 11. 02. 18:45, Andre Przywara wrote: > On Sat, 2 Nov 2024 10:31:41 +0100 > "Csókás, Bence" <csokas.bence@prolan.hu> wrote: > > Hi, > >> From: Mesih Kilinc <mesihkilinc@gmail.com> >> >> static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) >> @@ -1215,6 +1218,13 @@ static int sun4i_dma_probe(struct platform_device *pdev) >> return PTR_ERR(priv->clk); >> } >> >> + if (priv->cfg->has_reset) { >> + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); > > Can't we use devm_reset_control_get_optional_exclusive(), and then save > this whole has_reset bit? For suniv, reset is REQUIRED. For sun4i, reset DOES NOT EXIST. has_reset does not mean that whether this instance has a reset control or not, that is handled by checking priv->rst for NULL. has_reset means whether reset is REQUIRED by this type of DMA, specified by the DT match data. Bence
diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c index b2c1e4b9f696..9d1e3c51342d 100644 --- a/drivers/dma/sun4i-dma.c +++ b/drivers/dma/sun4i-dma.c @@ -15,6 +15,7 @@ #include <linux/of_dma.h> #include <linux/of_device.h> #include <linux/platform_device.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -159,6 +160,7 @@ struct sun4i_dma_config { u8 ddma_drq_sdram; u8 max_burst; + bool has_reset; }; struct sun4i_dma_pchan { @@ -208,6 +210,7 @@ struct sun4i_dma_dev { int irq; spinlock_t lock; const struct sun4i_dma_config *cfg; + struct reset_control *rst; }; static struct sun4i_dma_dev *to_sun4i_dma_dev(struct dma_device *dev) @@ -1215,6 +1218,13 @@ static int sun4i_dma_probe(struct platform_device *pdev) return PTR_ERR(priv->clk); } + if (priv->cfg->has_reset) { + priv->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); + if (IS_ERR(priv->rst)) + return dev_err_probe(&pdev->dev, PTR_ERR(priv->rst), + "Failed to get reset control\n"); + } + platform_set_drvdata(pdev, priv); spin_lock_init(&priv->lock); @@ -1287,6 +1297,14 @@ static int sun4i_dma_probe(struct platform_device *pdev) return ret; } + /* Deassert the reset control */ + ret = reset_control_deassert(priv->rst); + if (ret) { + dev_err_probe(&pdev->dev, ret, + "Failed to deassert the reset control\n"); + goto err_clk_disable; + } + /* * Make sure the IRQs are all disabled and accounted for. The bootloader * likes to leave these dirty @@ -1355,6 +1373,7 @@ static struct sun4i_dma_config sun4i_a10_dma_cfg = { .ddma_drq_sdram = SUN4I_DDMA_DRQ_TYPE_SDRAM, .max_burst = SUN4I_MAX_BURST, + .has_reset = false, }; static const struct of_device_id sun4i_dma_match[] = {