| Message ID | 20230322094820.24738-3-walker.chen@starfivetech.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 790f3c8b8f9f63b1f5a3ffd06630ed3d0df9804c |
| Headers | show |
| Series | Add DMA driver for StarFive JH7110 SoC | expand |
On Wed, 22 Mar 2023 at 10:48, Walker Chen <walker.chen@starfivetech.com> wrote: > > Add DMA reset operation in device probe and use different configuration > on CH_CFG registers according to match data. Update all uses of > of_device_is_compatible with of_device_get_match_data. > > Signed-off-by: Walker Chen <walker.chen@starfivetech.com> Thanks! Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > --- > .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 38 ++++++++++++++++--- > drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + > 2 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 4169e1d7d5ca..6cfcb541d8c3 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -21,10 +21,12 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/of_dma.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/property.h> > +#include <linux/reset.h> > #include <linux/slab.h> > #include <linux/types.h> > > @@ -46,6 +48,10 @@ > DMA_SLAVE_BUSWIDTH_32_BYTES | \ > DMA_SLAVE_BUSWIDTH_64_BYTES) > > +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0) > +#define AXI_DMA_FLAG_HAS_RESETS BIT(1) > +#define AXI_DMA_FLAG_USE_CFG2 BIT(2) > + > static inline void > axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val) > { > @@ -86,7 +92,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, > > cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | > config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); > - if (chan->chip->dw->hdata->reg_map_8_channels) { > + if (chan->chip->dw->hdata->reg_map_8_channels && > + !chan->chip->dw->hdata->use_cfg2) { > cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS | > config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS | > config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS | > @@ -1367,10 +1374,11 @@ static int parse_device_properties(struct axi_dma_chip *chip) > > static int dw_probe(struct platform_device *pdev) > { > - struct device_node *node = pdev->dev.of_node; > struct axi_dma_chip *chip; > struct dw_axi_dma *dw; > struct dw_axi_dma_hcfg *hdata; > + struct reset_control *resets; > + unsigned int flags; > u32 i; > int ret; > > @@ -1398,12 +1406,25 @@ static int dw_probe(struct platform_device *pdev) > if (IS_ERR(chip->regs)) > return PTR_ERR(chip->regs); > > - if (of_device_is_compatible(node, "intel,kmb-axi-dma")) { > + flags = (uintptr_t)of_device_get_match_data(&pdev->dev); > + if (flags & AXI_DMA_FLAG_HAS_APB_REGS) { > chip->apb_regs = devm_platform_ioremap_resource(pdev, 1); > if (IS_ERR(chip->apb_regs)) > return PTR_ERR(chip->apb_regs); > } > > + if (flags & AXI_DMA_FLAG_HAS_RESETS) { > + resets = devm_reset_control_array_get_exclusive(&pdev->dev); > + if (IS_ERR(resets)) > + return PTR_ERR(resets); > + > + ret = reset_control_deassert(resets); > + if (ret) > + return ret; > + } > + > + chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2); > + > chip->core_clk = devm_clk_get(chip->dev, "core-clk"); > if (IS_ERR(chip->core_clk)) > return PTR_ERR(chip->core_clk); > @@ -1554,8 +1575,15 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = { > }; > > static const struct of_device_id dw_dma_of_id_table[] = { > - { .compatible = "snps,axi-dma-1.01a" }, > - { .compatible = "intel,kmb-axi-dma" }, > + { > + .compatible = "snps,axi-dma-1.01a" > + }, { > + .compatible = "intel,kmb-axi-dma", > + .data = (void *)AXI_DMA_FLAG_HAS_APB_REGS, > + }, { > + .compatible = "starfive,jh7110-axi-dma", > + .data = (void *)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2), > + }, > {} > }; > MODULE_DEVICE_TABLE(of, dw_dma_of_id_table); > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > index e9d5eb0fd594..eb267cb24f67 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > @@ -33,6 +33,7 @@ struct dw_axi_dma_hcfg { > /* Register map for DMAX_NUM_CHANNELS <= 8 */ > bool reg_map_8_channels; > bool restrict_axi_burst_len; > + bool use_cfg2; > }; > > struct axi_dma_chan { > -- > 2.17.1 >
On 22-03-23, 17:48, Walker Chen wrote: > Add DMA reset operation in device probe and use different configuration > on CH_CFG registers according to match data. Update all uses of > of_device_is_compatible with of_device_get_match_data. > > Signed-off-by: Walker Chen <walker.chen@starfivetech.com> > --- > .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 38 ++++++++++++++++--- > drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + > 2 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 4169e1d7d5ca..6cfcb541d8c3 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -21,10 +21,12 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/of_dma.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/property.h> > +#include <linux/reset.h> > #include <linux/slab.h> > #include <linux/types.h> > > @@ -46,6 +48,10 @@ > DMA_SLAVE_BUSWIDTH_32_BYTES | \ > DMA_SLAVE_BUSWIDTH_64_BYTES) > > +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0) > +#define AXI_DMA_FLAG_HAS_RESETS BIT(1) > +#define AXI_DMA_FLAG_USE_CFG2 BIT(2) > + > static inline void > axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val) > { > @@ -86,7 +92,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, > > cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | > config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); > - if (chan->chip->dw->hdata->reg_map_8_channels) { > + if (chan->chip->dw->hdata->reg_map_8_channels && > + !chan->chip->dw->hdata->use_cfg2) { I think this will break existing users.. This is set for reg_map_8_channels && use_cfg2, latter being set only for new controller, so what about existing users of these bits? > cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS | > config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS | > config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS | > @@ -1367,10 +1374,11 @@ static int parse_device_properties(struct axi_dma_chip *chip) > > static int dw_probe(struct platform_device *pdev) > { > - struct device_node *node = pdev->dev.of_node; > struct axi_dma_chip *chip; > struct dw_axi_dma *dw; > struct dw_axi_dma_hcfg *hdata; > + struct reset_control *resets; > + unsigned int flags; > u32 i; > int ret; > > @@ -1398,12 +1406,25 @@ static int dw_probe(struct platform_device *pdev) > if (IS_ERR(chip->regs)) > return PTR_ERR(chip->regs); > > - if (of_device_is_compatible(node, "intel,kmb-axi-dma")) { > + flags = (uintptr_t)of_device_get_match_data(&pdev->dev); > + if (flags & AXI_DMA_FLAG_HAS_APB_REGS) { > chip->apb_regs = devm_platform_ioremap_resource(pdev, 1); > if (IS_ERR(chip->apb_regs)) > return PTR_ERR(chip->apb_regs); > } > > + if (flags & AXI_DMA_FLAG_HAS_RESETS) { > + resets = devm_reset_control_array_get_exclusive(&pdev->dev); > + if (IS_ERR(resets)) > + return PTR_ERR(resets); > + > + ret = reset_control_deassert(resets); > + if (ret) > + return ret; > + } > + > + chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2); > + > chip->core_clk = devm_clk_get(chip->dev, "core-clk"); > if (IS_ERR(chip->core_clk)) > return PTR_ERR(chip->core_clk); > @@ -1554,8 +1575,15 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = { > }; > > static const struct of_device_id dw_dma_of_id_table[] = { > - { .compatible = "snps,axi-dma-1.01a" }, > - { .compatible = "intel,kmb-axi-dma" }, > + { > + .compatible = "snps,axi-dma-1.01a" > + }, { > + .compatible = "intel,kmb-axi-dma", > + .data = (void *)AXI_DMA_FLAG_HAS_APB_REGS, > + }, { > + .compatible = "starfive,jh7110-axi-dma", > + .data = (void *)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2), > + }, > {} > }; > MODULE_DEVICE_TABLE(of, dw_dma_of_id_table); > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > index e9d5eb0fd594..eb267cb24f67 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > @@ -33,6 +33,7 @@ struct dw_axi_dma_hcfg { > /* Register map for DMAX_NUM_CHANNELS <= 8 */ > bool reg_map_8_channels; > bool restrict_axi_burst_len; > + bool use_cfg2; > }; > > struct axi_dma_chan { > -- > 2.17.1
On 2023/3/31 20:04, Vinod Koul wrote: > On 22-03-23, 17:48, Walker Chen wrote: >> Add DMA reset operation in device probe and use different configuration >> on CH_CFG registers according to match data. Update all uses of >> of_device_is_compatible with of_device_get_match_data. >> >> Signed-off-by: Walker Chen <walker.chen@starfivetech.com> >> --- >> .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 38 ++++++++++++++++--- >> drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + >> 2 files changed, 34 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c >> index 4169e1d7d5ca..6cfcb541d8c3 100644 >> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c >> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c >> @@ -21,10 +21,12 @@ >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/of.h> >> +#include <linux/of_device.h> >> #include <linux/of_dma.h> >> #include <linux/platform_device.h> >> #include <linux/pm_runtime.h> >> #include <linux/property.h> >> +#include <linux/reset.h> >> #include <linux/slab.h> >> #include <linux/types.h> >> >> @@ -46,6 +48,10 @@ >> DMA_SLAVE_BUSWIDTH_32_BYTES | \ >> DMA_SLAVE_BUSWIDTH_64_BYTES) >> >> +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0) >> +#define AXI_DMA_FLAG_HAS_RESETS BIT(1) >> +#define AXI_DMA_FLAG_USE_CFG2 BIT(2) >> + >> static inline void >> axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val) >> { >> @@ -86,7 +92,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, >> >> cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | >> config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); >> - if (chan->chip->dw->hdata->reg_map_8_channels) { >> + if (chan->chip->dw->hdata->reg_map_8_channels && >> + !chan->chip->dw->hdata->use_cfg2) { > > I think this will break existing users.. > > This is set for reg_map_8_channels && use_cfg2, latter being set only > for new controller, so what about existing users of these bits? Firstly thank you for your comments! There is a statement 'use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);' to be added in dw_probe function. Assuming older/existing platform run this code block, e.g. when compatible is "snps,axi-dma-1.01a", the value of variable 'use_cfg2' is still false, the original logic will not be broken. So other existing users are not affected by this. Looking forward to your more comments. Thanks! Best regards, Walker
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index 4169e1d7d5ca..6cfcb541d8c3 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -21,10 +21,12 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_device.h> #include <linux/of_dma.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/property.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/types.h> @@ -46,6 +48,10 @@ DMA_SLAVE_BUSWIDTH_32_BYTES | \ DMA_SLAVE_BUSWIDTH_64_BYTES) +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0) +#define AXI_DMA_FLAG_HAS_RESETS BIT(1) +#define AXI_DMA_FLAG_USE_CFG2 BIT(2) + static inline void axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val) { @@ -86,7 +92,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan, cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS | config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS); - if (chan->chip->dw->hdata->reg_map_8_channels) { + if (chan->chip->dw->hdata->reg_map_8_channels && + !chan->chip->dw->hdata->use_cfg2) { cfg_hi = config->tt_fc << CH_CFG_H_TT_FC_POS | config->hs_sel_src << CH_CFG_H_HS_SEL_SRC_POS | config->hs_sel_dst << CH_CFG_H_HS_SEL_DST_POS | @@ -1367,10 +1374,11 @@ static int parse_device_properties(struct axi_dma_chip *chip) static int dw_probe(struct platform_device *pdev) { - struct device_node *node = pdev->dev.of_node; struct axi_dma_chip *chip; struct dw_axi_dma *dw; struct dw_axi_dma_hcfg *hdata; + struct reset_control *resets; + unsigned int flags; u32 i; int ret; @@ -1398,12 +1406,25 @@ static int dw_probe(struct platform_device *pdev) if (IS_ERR(chip->regs)) return PTR_ERR(chip->regs); - if (of_device_is_compatible(node, "intel,kmb-axi-dma")) { + flags = (uintptr_t)of_device_get_match_data(&pdev->dev); + if (flags & AXI_DMA_FLAG_HAS_APB_REGS) { chip->apb_regs = devm_platform_ioremap_resource(pdev, 1); if (IS_ERR(chip->apb_regs)) return PTR_ERR(chip->apb_regs); } + if (flags & AXI_DMA_FLAG_HAS_RESETS) { + resets = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(resets)) + return PTR_ERR(resets); + + ret = reset_control_deassert(resets); + if (ret) + return ret; + } + + chip->dw->hdata->use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2); + chip->core_clk = devm_clk_get(chip->dev, "core-clk"); if (IS_ERR(chip->core_clk)) return PTR_ERR(chip->core_clk); @@ -1554,8 +1575,15 @@ static const struct dev_pm_ops dw_axi_dma_pm_ops = { }; static const struct of_device_id dw_dma_of_id_table[] = { - { .compatible = "snps,axi-dma-1.01a" }, - { .compatible = "intel,kmb-axi-dma" }, + { + .compatible = "snps,axi-dma-1.01a" + }, { + .compatible = "intel,kmb-axi-dma", + .data = (void *)AXI_DMA_FLAG_HAS_APB_REGS, + }, { + .compatible = "starfive,jh7110-axi-dma", + .data = (void *)(AXI_DMA_FLAG_HAS_RESETS | AXI_DMA_FLAG_USE_CFG2), + }, {} }; MODULE_DEVICE_TABLE(of, dw_dma_of_id_table); diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h index e9d5eb0fd594..eb267cb24f67 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -33,6 +33,7 @@ struct dw_axi_dma_hcfg { /* Register map for DMAX_NUM_CHANNELS <= 8 */ bool reg_map_8_channels; bool restrict_axi_burst_len; + bool use_cfg2; }; struct axi_dma_chan {
Add DMA reset operation in device probe and use different configuration on CH_CFG registers according to match data. Update all uses of of_device_is_compatible with of_device_get_match_data. Signed-off-by: Walker Chen <walker.chen@starfivetech.com> --- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 38 ++++++++++++++++--- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 + 2 files changed, 34 insertions(+), 5 deletions(-)