| Message ID | 20190123152310.11680-5-angus@akkea.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] dma: imx-sdma: add clock ratio 1:1 check | expand |
Hi Angus, Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism): > On i.mx8mq, there are two sdma instances, and the common dma framework > will get a channel dynamically from any available sdma instance whether > it's the first sdma device or the second sdma device. Some IPs like > SAI only work with sdma2 not sdma1. To make sure the sdma channel is from > the correct sdma device, use an index to match. > > Based on MLK-16104-2 by Robin Gong <yibin.gong@nxp.com> This relies on the probe order of the devices (which should be treated as random) for the match to find the right device. This is not acceptable upstream. The DT "dmas" property already specifies the correct SDMA device to use for a consumer, so the filter function should really match the OF node of the SDMA device with the node specified in the dmas phandle in order to pick the right SDMA engine. Regards, Lucas > > > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca> > --- > drivers/dma/imx-sdma.c | 12 ++++++++++++ > include/linux/platform_data/dma-imx.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c > index 2e691b1cd0eb..bf3752a6a64f 100644 > --- a/drivers/dma/imx-sdma.c > +++ b/drivers/dma/imx-sdma.c > @@ -442,6 +442,7 @@ struct sdma_engine { > > > struct sdma_buffer_descriptor *bd0; > > /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/ > > > bool clk_ratio; > > > + int idx; > }; > > static int sdma_config_write(struct dma_chan *chan, > @@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = { > }; > MODULE_DEVICE_TABLE(of, sdma_dt_ids); > > +static int sdma_dev_idx; > + > > #define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */ > > #define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */ > > #define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */ > @@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param) > > if (!imx_dma_is_general_purpose(chan)) > > return false; > > > + /* return false if it's not the right device */ > > + if ((sdmac->sdma->drvdata == &sdma_imx8mq) > > + && (sdmac->sdma->idx != data->idx)) > > + return false; > + > > sdmac->data = *data; > > chan->private = &sdmac->data; > > @@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec, > > * be set to sdmac->event_id1. > > */ > > data.dma_request2 = 0; > > + data.idx = sdma->idx; > > > return dma_request_channel(mask, sdma_filter_fn, &data); > } > @@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device *pdev) > > of_node_put(spba_bus); > > } > > > + /* There maybe multi sdma devices such as i.mx8mq */ > > + sdma->idx = sdma_dev_idx++; > + > > return 0; > > err_register: > diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h > index 7d964e787299..843faf081282 100644 > --- a/include/linux/platform_data/dma-imx.h > +++ b/include/linux/platform_data/dma-imx.h > @@ -55,6 +55,7 @@ struct imx_dma_data { > > int dma_request2; /* secondary DMA request line */ > > enum sdma_peripheral_type peripheral_type; > > int priority; > > + int idx; > }; > > static inline int imx_dma_is_ipu(struct dma_chan *chan)
On 2019-01-23 08:31, Lucas Stach wrote: > Hi Angus, > > Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie > (Purism): >> On i.mx8mq, there are two sdma instances, and the common dma framework >> will get a channel dynamically from any available sdma instance >> whether >> it's the first sdma device or the second sdma device. Some IPs like >> SAI only work with sdma2 not sdma1. To make sure the sdma channel is >> from >> the correct sdma device, use an index to match. >> >> Based on MLK-16104-2 by Robin Gong <yibin.gong@nxp.com> > > This relies on the probe order of the devices (which should be treated > as random) for the match to find the right device. This is not > acceptable upstream. > > The DT "dmas" property already specifies the correct SDMA device to use > for a consumer, so the filter function should really match the OF node > of the SDMA device with the node specified in the dmas phandle in order > to pick the right SDMA engine. > Thanks Lucas, I'll fix it for rev 4 > Regards, > Lucas > >> >> > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca> >> --- >> drivers/dma/imx-sdma.c | 12 ++++++++++++ >> include/linux/platform_data/dma-imx.h | 1 + >> 2 files changed, 13 insertions(+) >> >> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c >> index 2e691b1cd0eb..bf3752a6a64f 100644 >> --- a/drivers/dma/imx-sdma.c >> +++ b/drivers/dma/imx-sdma.c >> @@ -442,6 +442,7 @@ struct sdma_engine { >> > > struct sdma_buffer_descriptor *bd0; >> > /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/ >> > > bool clk_ratio; >> > > + int idx; >> }; >> >> static int sdma_config_write(struct dma_chan *chan, >> @@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = { >> }; >> MODULE_DEVICE_TABLE(of, sdma_dt_ids); >> >> +static int sdma_dev_idx; >> + >> > #define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */ >> > #define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */ >> > #define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */ >> @@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan >> *chan, void *fn_param) >> > if (!imx_dma_is_general_purpose(chan)) >> > return false; >> >> > + /* return false if it's not the right device */ >> > + if ((sdmac->sdma->drvdata == &sdma_imx8mq) >> > + && (sdmac->sdma->idx != data->idx)) >> > + return false; >> + >> > sdmac->data = *data; >> > chan->private = &sdmac->data; >> >> @@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct >> of_phandle_args *dma_spec, >> > * be set to sdmac->event_id1. >> > */ >> > data.dma_request2 = 0; >> > + data.idx = sdma->idx; >> >> > return dma_request_channel(mask, sdma_filter_fn, &data); >> } >> @@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device >> *pdev) >> > of_node_put(spba_bus); >> > } >> >> > + /* There maybe multi sdma devices such as i.mx8mq */ >> > + sdma->idx = sdma_dev_idx++; >> + >> > return 0; >> >> err_register: >> diff --git a/include/linux/platform_data/dma-imx.h >> b/include/linux/platform_data/dma-imx.h >> index 7d964e787299..843faf081282 100644 >> --- a/include/linux/platform_data/dma-imx.h >> +++ b/include/linux/platform_data/dma-imx.h >> @@ -55,6 +55,7 @@ struct imx_dma_data { >> > int dma_request2; /* secondary DMA request line */ >> > enum sdma_peripheral_type peripheral_type; >> > int priority; >> > + int idx; >> }; >> >> static inline int imx_dma_is_ipu(struct dma_chan *chan)
On 23-01-19, 08:42, Angus Ainslie wrote: > On 2019-01-23 08:31, Lucas Stach wrote: > > Hi Angus, > > > > Am Mittwoch, den 23.01.2019, 08:23 -0700 schrieb Angus Ainslie (Purism): > > > On i.mx8mq, there are two sdma instances, and the common dma framework > > > will get a channel dynamically from any available sdma instance > > > whether > > > it's the first sdma device or the second sdma device. Some IPs like > > > SAI only work with sdma2 not sdma1. To make sure the sdma channel is > > > from > > > the correct sdma device, use an index to match. > > > > > > Based on MLK-16104-2 by Robin Gong <yibin.gong@nxp.com> > > > > This relies on the probe order of the devices (which should be treated > > as random) for the match to find the right device. This is not > > acceptable upstream. > > > > The DT "dmas" property already specifies the correct SDMA device to use > > for a consumer, so the filter function should really match the OF node > > of the SDMA device with the node specified in the dmas phandle in order > > to pick the right SDMA engine. > > > > Thanks Lucas, I'll fix it for rev 4 And fix the subsystem name, it is 'dmaengine: ...' not 'dma: ...'. git log of the subsystem should give you good clue of the tags to be used.. Thanks
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 2e691b1cd0eb..bf3752a6a64f 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -442,6 +442,7 @@ struct sdma_engine { struct sdma_buffer_descriptor *bd0; /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/ bool clk_ratio; + int idx; }; static int sdma_config_write(struct dma_chan *chan, @@ -606,6 +607,8 @@ static const struct of_device_id sdma_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, sdma_dt_ids); +static int sdma_dev_idx; + #define SDMA_H_CONFIG_DSPDMA BIT(12) /* indicates if the DSPDMA is used */ #define SDMA_H_CONFIG_RTD_PINS BIT(11) /* indicates if Real-Time Debug pins are enabled */ #define SDMA_H_CONFIG_ACR BIT(4) /* indicates if AHB freq /core freq = 2 or 1 */ @@ -1934,6 +1937,11 @@ static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param) if (!imx_dma_is_general_purpose(chan)) return false; + /* return false if it's not the right device */ + if ((sdmac->sdma->drvdata == &sdma_imx8mq) + && (sdmac->sdma->idx != data->idx)) + return false; + sdmac->data = *data; chan->private = &sdmac->data; @@ -1961,6 +1969,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec, * be set to sdmac->event_id1. */ data.dma_request2 = 0; + data.idx = sdma->idx; return dma_request_channel(mask, sdma_filter_fn, &data); } @@ -2149,6 +2158,9 @@ static int sdma_probe(struct platform_device *pdev) of_node_put(spba_bus); } + /* There maybe multi sdma devices such as i.mx8mq */ + sdma->idx = sdma_dev_idx++; + return 0; err_register: diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h index 7d964e787299..843faf081282 100644 --- a/include/linux/platform_data/dma-imx.h +++ b/include/linux/platform_data/dma-imx.h @@ -55,6 +55,7 @@ struct imx_dma_data { int dma_request2; /* secondary DMA request line */ enum sdma_peripheral_type peripheral_type; int priority; + int idx; }; static inline int imx_dma_is_ipu(struct dma_chan *chan)
On i.mx8mq, there are two sdma instances, and the common dma framework will get a channel dynamically from any available sdma instance whether it's the first sdma device or the second sdma device. Some IPs like SAI only work with sdma2 not sdma1. To make sure the sdma channel is from the correct sdma device, use an index to match. Based on MLK-16104-2 by Robin Gong <yibin.gong@nxp.com> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca> --- drivers/dma/imx-sdma.c | 12 ++++++++++++ include/linux/platform_data/dma-imx.h | 1 + 2 files changed, 13 insertions(+)