| Message ID | 20210819121137.11928-1-tangbin@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | power: supply: cpcap-battery: remove redundant check | expand |
Hi, On Thu, Aug 19, 2021 at 08:11:37PM +0800, Tang Bin wrote: > In the function cpcap_battery_probe(), the check of '!match->data' > can actually never happen for the driver. First, this probe function > will only be called if there is a match with an entry from the OF > device ID table, and then all entries have .data set to a valid point. > So remove the redundant check. > > Co-developed-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com> > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> > --- NAK. Instead replace the complate of_match_device() part. The driver only needs the data object and there is of_device_get_match_data() for this. Also - even better - there is a non-DT specific variant which is device_get_match_data(). Please use that: const struct cpcap_battery_config *cfg = device_get_match_data(&pdev->dev); if (!cfg) return -ENODEV; memcpy(&ddata->config, cfg, sizeof(ddata->config)); Thanks, -- Sebastian > drivers/power/supply/cpcap-battery.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c > index 8d62d4241..a3866826b 100644 > --- a/drivers/power/supply/cpcap-battery.c > +++ b/drivers/power/supply/cpcap-battery.c > @@ -1035,12 +1035,6 @@ static int cpcap_battery_probe(struct platform_device *pdev) > if (!match) > return -EINVAL; > > - if (!match->data) { > - dev_err(&pdev->dev, "no configuration data found\n"); > - > - return -ENODEV; > - } > - > ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); > if (!ddata) > return -ENOMEM; > -- > 2.20.1.windows.1 > > >
Hi, Reichel On 2021/9/27 23:46, Sebastian Reichel wrote: > NAK. Instead replace the complate of_match_device() part. The driver > only needs the data object and there is of_device_get_match_data() > for this. Also - even better - there is a non-DT specific variant > which is device_get_match_data(). Please use that: > > > const struct cpcap_battery_config *cfg = device_get_match_data(&pdev->dev); > if (!cfg) > return -ENODEV; > > memcpy(&ddata->config, cfg, sizeof(ddata->config)); > Got it. Before this patch, I thought it would be better to use of_device_get_match_data() to simplify for this place, but your email made me learn it again. I will send patch v2 for you. Thanks Tang Bin > >> drivers/power/supply/cpcap-battery.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c >> index 8d62d4241..a3866826b 100644 >> --- a/drivers/power/supply/cpcap-battery.c >> +++ b/drivers/power/supply/cpcap-battery.c >> @@ -1035,12 +1035,6 @@ static int cpcap_battery_probe(struct platform_device *pdev) >> if (!match) >> return -EINVAL; >> >> - if (!match->data) { >> - dev_err(&pdev->dev, "no configuration data found\n"); >> - >> - return -ENODEV; >> - } >> - >> ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); >> if (!ddata) >> return -ENOMEM; >> -- >> 2.20.1.windows.1 >> >> >>
diff --git a/drivers/power/supply/cpcap-battery.c b/drivers/power/supply/cpcap-battery.c index 8d62d4241..a3866826b 100644 --- a/drivers/power/supply/cpcap-battery.c +++ b/drivers/power/supply/cpcap-battery.c @@ -1035,12 +1035,6 @@ static int cpcap_battery_probe(struct platform_device *pdev) if (!match) return -EINVAL; - if (!match->data) { - dev_err(&pdev->dev, "no configuration data found\n"); - - return -ENODEV; - } - ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); if (!ddata) return -ENOMEM;