| Message ID | 1617008316-60426-1-git-send-email-tiantao6@hisilicon.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: davinci: Use of_device_get_match_data() helper | expand |
在 2021/3/29 23:21, Mark Brown 写道: > On Mon, Mar 29, 2021 at 04:58:36PM +0800, Tian Tao wrote: > >> + spi_data = (struct davinci_spi_of_data *) >> + of_device_get_match_data(&pdev->dev); >> + if (!spi_data) > If we need to cast away from void * that's most likely pointing at a > bug. Even with of_match_device, there is still a type conversion，as follows spi_data = (struct davinci_spi_of_data *)match->data; Using of_device_get_match_data instead of of_match_device doesn't look like a problem from the code, other spi drivers do the same thing
On Tue, Mar 30, 2021 at 08:53:00AM +0800, tiantao (H) wrote: > 在 2021/3/29 23:21, Mark Brown 写道: > > On Mon, Mar 29, 2021 at 04:58:36PM +0800, Tian Tao wrote: > > > + spi_data = (struct davinci_spi_of_data *) > > > + of_device_get_match_data(&pdev->dev); > > > + if (!spi_data) > > If we need to cast away from void * that's most likely pointing at a > > bug. > Even with of_match_device, there is still a type conversion，as follows > spi_data = (struct davinci_spi_of_data *)match->data; A type conversion which shouldn't have any need for a cast.
On Mon, Mar 29, 2021 at 9:54 PM tiantao (H) <tiantao6@huawei.com> wrote: > Even with of_match_device, there is still a type conversion，as follows > > spi_data = (struct davinci_spi_of_data *)match->data; > > Using of_device_get_match_data instead of of_match_device doesn't look > like a problem from the code, other spi drivers do the same thing What about doing it like this? --- a/drivers/spi/spi-davinci.c +++ b/drivers/spi/spi-davinci.c @@ -817,18 +817,13 @@ static int spi_davinci_get_pdata(struct platform_device *pdev, struct davinci_spi *dspi) { struct device_node *node = pdev->dev.of_node; - struct davinci_spi_of_data *spi_data; + const struct davinci_spi_of_data *spi_data; struct davinci_spi_platform_data *pdata; unsigned int num_cs, intr_line = 0; - const struct of_device_id *match; pdata = &dspi->pdata; - match = of_match_device(davinci_spi_of_match, &pdev->dev); - if (!match) - return -ENODEV; - - spi_data = (struct davinci_spi_of_data *)match->data; + spi_data = device_get_match_data(&pdev->dev); pdata->version = spi_data->version; pdata->prescaler_limit = spi_data->prescaler_limit;
在 2021/3/30 19:56, Fabio Estevam 写道: > On Mon, Mar 29, 2021 at 9:54 PM tiantao (H) <tiantao6@huawei.com> wrote: > >> Even with of_match_device, there is still a type conversion，as follows >> >> spi_data = (struct davinci_spi_of_data *)match->data; >> >> Using of_device_get_match_data instead of of_match_device doesn't look >> like a problem from the code, other spi drivers do the same thing > What about doing it like this? thanks. From what I have tested, the results are ok. I will send v2 to fix. make modules M=drivers/spi/ CC [M] drivers/spi//spi-davinci.o drivers/spi//spi-davinci.c: In function ‘spi_davinci_get_pdata’: drivers/spi//spi-davinci.c:826:11: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 826 | spi_data = of_device_get_match_data(&pdev->dev); | ^ MODPOST drivers/spi//Module.symvers LD [M] drivers/spi//spi-davinci.ko vim drivers/spi//spi-davinci.c make modules M=drivers/spi/ CC [M] drivers/spi//spi-davinci.o MODPOST drivers/spi//Module.symvers LD [M] drivers/spi//spi-davinci.ko > > --- a/drivers/spi/spi-davinci.c > +++ b/drivers/spi/spi-davinci.c > @@ -817,18 +817,13 @@ static int spi_davinci_get_pdata(struct > platform_device *pdev, > struct davinci_spi *dspi) > { > struct device_node *node = pdev->dev.of_node; > - struct davinci_spi_of_data *spi_data; > + const struct davinci_spi_of_data *spi_data; > struct davinci_spi_platform_data *pdata; > unsigned int num_cs, intr_line = 0; > - const struct of_device_id *match; > > pdata = &dspi->pdata; > > - match = of_match_device(davinci_spi_of_match, &pdev->dev); > - if (!match) > - return -ENODEV; > - > - spi_data = (struct davinci_spi_of_data *)match->data; > + spi_data = device_get_match_data(&pdev->dev); > > pdata->version = spi_data->version; > pdata->prescaler_limit = spi_data->prescaler_limit; > . >
On Tue, Mar 30, 2021 at 08:56:41AM -0300, Fabio Estevam wrote: > What about doing it like this? > - spi_data = (struct davinci_spi_of_data *)match->data; > + spi_data = device_get_match_data(&pdev->dev); Yes, exactly.
On Tue, Mar 30, 2021 at 9:12 AM tiantao (H) <tiantao6@huawei.com> wrote: > From what I have tested, the results are ok. I will send v2 to fix. > > make modules M=drivers/spi/ > CC [M] drivers/spi//spi-davinci.o > drivers/spi//spi-davinci.c: In function ‘spi_davinci_get_pdata’: > drivers/spi//spi-davinci.c:826:11: warning: assignment discards ‘const’ > qualifier from pointer target type [-Wdiscarded-qualifiers] > 826 | spi_data = of_device_get_match_data(&pdev->dev); This warning does not appear if you apply the patch I suggested.
在 2021/3/30 20:15, Fabio Estevam 写道: > On Tue, Mar 30, 2021 at 9:12 AM tiantao (H) <tiantao6@huawei.com> wrote: > >> From what I have tested, the results are ok. I will send v2 to fix. >> >> make modules M=drivers/spi/ >> CC [M] drivers/spi//spi-davinci.o >> drivers/spi//spi-davinci.c: In function ‘spi_davinci_get_pdata’: >> drivers/spi//spi-davinci.c:826:11: warning: assignment discards ‘const’ >> qualifier from pointer target type [-Wdiscarded-qualifiers] >> 826 | spi_data = of_device_get_match_data(&pdev->dev); Yes, I added the const as you suggested and there is no more warning > This warning does not appear if you apply the patch I suggested. > . >
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c index 7453a1d..869045f 100644 --- a/drivers/spi/spi-davinci.c +++ b/drivers/spi/spi-davinci.c @@ -820,16 +820,14 @@ static int spi_davinci_get_pdata(struct platform_device *pdev, struct davinci_spi_of_data *spi_data; struct davinci_spi_platform_data *pdata; unsigned int num_cs, intr_line = 0; - const struct of_device_id *match; pdata = &dspi->pdata; - match = of_match_device(davinci_spi_of_match, &pdev->dev); - if (!match) + spi_data = (struct davinci_spi_of_data *) + of_device_get_match_data(&pdev->dev); + if (!spi_data) return -ENODEV; - spi_data = (struct davinci_spi_of_data *)match->data; - pdata->version = spi_data->version; pdata->prescaler_limit = spi_data->prescaler_limit; /*
Use the of_device_get_match_data() helper instead of open coding. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/spi/spi-davinci.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)