| Message ID | 20230221140424.719-3-walker.chen@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add DMA driver for StarFive JH7110 SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Tue, 21 Feb 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote: > > Add DMA reset operation in device probe and use different configuration > on CH_CFG registers according to compatible string. > > Signed-off-by: Walker Chen <walker.chen@starfivetech.com> > --- > .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 19 +++++++++++++++++-- > drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 3 +++ > 2 files changed, 20 insertions(+), 2 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 bf85aa0979ec..858c4337650f 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -25,6 +25,7 @@ > #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> > > @@ -86,7 +87,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 | > @@ -1142,7 +1144,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) > axi_chan_disable(chan); > > ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val, > - !(val & chan_active), 1000, 10000); > + !(val & chan_active), 1000, DMAC_TIMEOUT_US); > if (ret == -ETIMEDOUT) > dev_warn(dchan2dev(dchan), > "%s failed to stop\n", axi_chan_name(chan)); > @@ -1416,6 +1418,18 @@ static int dw_probe(struct platform_device *pdev) > if (IS_ERR(chip->cfgr_clk)) > return PTR_ERR(chip->cfgr_clk); > > + if (of_device_is_compatible(node, "starfive,jh7110-axi-dma")) { > + chip->resets = devm_reset_control_array_get_exclusive(&pdev->dev); > + if (IS_ERR(chip->resets)) > + return PTR_ERR(chip->resets); > + > + ret = reset_control_deassert(chip->resets); > + if (ret) > + return ret; > + > + chip->dw->hdata->use_cfg2 = true; > + } > + In the future it would be great to use match data rather than all the calls to of_device_is_compatible. > ret = parse_device_properties(chip); > if (ret) > return ret; > @@ -1560,6 +1574,7 @@ 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 = "starfive,jh7110-axi-dma" }, > {} > }; > 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..761d95691c02 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h > @@ -21,6 +21,7 @@ > #define DMAC_MAX_CHANNELS 16 > #define DMAC_MAX_MASTERS 2 > #define DMAC_MAX_BLK_SIZE 0x200000 > +#define DMAC_TIMEOUT_US 200000 > > struct dw_axi_dma_hcfg { > u32 nr_channels; > @@ -33,6 +34,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 { > @@ -70,6 +72,7 @@ struct axi_dma_chip { > struct clk *core_clk; > struct clk *cfgr_clk; > struct dw_axi_dma *dw; > + struct reset_control *resets; This added field only seems to be written, but not read from anywhere. With that fixed: Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > }; > > /* LLI == Linked List Item */ > -- > 2.17.1 >
On 2023/2/26 22:25, Emil Renner Berthing wrote: > On Tue, 21 Feb 2023 at 15:04, Walker Chen <walker.chen@starfivetech.com> wrote: >> >> Add DMA reset operation in device probe and use different configuration >> on CH_CFG registers according to compatible string. >> >> Signed-off-by: Walker Chen <walker.chen@starfivetech.com> >> --- >> .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 19 +++++++++++++++++-- >> drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 3 +++ >> 2 files changed, 20 insertions(+), 2 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 bf85aa0979ec..858c4337650f 100644 >> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c >> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c >> @@ -25,6 +25,7 @@ >> #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> >> >> @@ -86,7 +87,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 | >> @@ -1142,7 +1144,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) >> axi_chan_disable(chan); >> >> ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val, >> - !(val & chan_active), 1000, 10000); >> + !(val & chan_active), 1000, DMAC_TIMEOUT_US); >> if (ret == -ETIMEDOUT) >> dev_warn(dchan2dev(dchan), >> "%s failed to stop\n", axi_chan_name(chan)); >> @@ -1416,6 +1418,18 @@ static int dw_probe(struct platform_device *pdev) >> if (IS_ERR(chip->cfgr_clk)) >> return PTR_ERR(chip->cfgr_clk); >> >> + if (of_device_is_compatible(node, "starfive,jh7110-axi-dma")) { >> + chip->resets = devm_reset_control_array_get_exclusive(&pdev->dev); >> + if (IS_ERR(chip->resets)) >> + return PTR_ERR(chip->resets); >> + >> + ret = reset_control_deassert(chip->resets); >> + if (ret) >> + return ret; >> + >> + chip->dw->hdata->use_cfg2 = true; >> + } >> + > > In the future it would be great to use match data rather than all the > calls to of_device_is_compatible. Thanks, about this point, I will follow your advice. > >> ret = parse_device_properties(chip); >> if (ret) >> return ret; >> @@ -1560,6 +1574,7 @@ 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 = "starfive,jh7110-axi-dma" }, >> {} >> }; >> 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..761d95691c02 100644 >> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h >> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h >> @@ -21,6 +21,7 @@ >> #define DMAC_MAX_CHANNELS 16 >> #define DMAC_MAX_MASTERS 2 >> #define DMAC_MAX_BLK_SIZE 0x200000 >> +#define DMAC_TIMEOUT_US 200000 >> >> struct dw_axi_dma_hcfg { >> u32 nr_channels; >> @@ -33,6 +34,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 { >> @@ -70,6 +72,7 @@ struct axi_dma_chip { >> struct clk *core_clk; >> struct clk *cfgr_clk; >> struct dw_axi_dma *dw; >> + struct reset_control *resets; > > This added field only seems to be written, but not read from anywhere. Reset initialization will be encapsulated in match data. > With that fixed: > > Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> Thanks for your review and comment very much! 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 bf85aa0979ec..858c4337650f 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -25,6 +25,7 @@ #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> @@ -86,7 +87,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 | @@ -1142,7 +1144,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) axi_chan_disable(chan); ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val, - !(val & chan_active), 1000, 10000); + !(val & chan_active), 1000, DMAC_TIMEOUT_US); if (ret == -ETIMEDOUT) dev_warn(dchan2dev(dchan), "%s failed to stop\n", axi_chan_name(chan)); @@ -1416,6 +1418,18 @@ static int dw_probe(struct platform_device *pdev) if (IS_ERR(chip->cfgr_clk)) return PTR_ERR(chip->cfgr_clk); + if (of_device_is_compatible(node, "starfive,jh7110-axi-dma")) { + chip->resets = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(chip->resets)) + return PTR_ERR(chip->resets); + + ret = reset_control_deassert(chip->resets); + if (ret) + return ret; + + chip->dw->hdata->use_cfg2 = true; + } + ret = parse_device_properties(chip); if (ret) return ret; @@ -1560,6 +1574,7 @@ 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 = "starfive,jh7110-axi-dma" }, {} }; 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..761d95691c02 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h @@ -21,6 +21,7 @@ #define DMAC_MAX_CHANNELS 16 #define DMAC_MAX_MASTERS 2 #define DMAC_MAX_BLK_SIZE 0x200000 +#define DMAC_TIMEOUT_US 200000 struct dw_axi_dma_hcfg { u32 nr_channels; @@ -33,6 +34,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 { @@ -70,6 +72,7 @@ struct axi_dma_chip { struct clk *core_clk; struct clk *cfgr_clk; struct dw_axi_dma *dw; + struct reset_control *resets; }; /* LLI == Linked List Item */
Add DMA reset operation in device probe and use different configuration on CH_CFG registers according to compatible string. Signed-off-by: Walker Chen <walker.chen@starfivetech.com> --- .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 19 +++++++++++++++++-- drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 3 +++ 2 files changed, 20 insertions(+), 2 deletions(-)