| Message ID | 20250309081607.27784-1-oushixiong1025@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] fbdev: fsl-diu-fb: add missing device_remove_file() | expand |
On 3/9/25 09:16, Shixiong Ou wrote: > From: Shixiong Ou <oushixiong@kylinos.cn> > > Call device_remove_file() when driver remove. > > Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn> > --- > v1->v2: > add has_sysfs_attrs flag. > > drivers/video/fbdev/fsl-diu-fb.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/video/fbdev/fsl-diu-fb.c b/drivers/video/fbdev/fsl-diu-fb.c > index 5ac8201c3533..57f7fe6a4c76 100644 > --- a/drivers/video/fbdev/fsl-diu-fb.c > +++ b/drivers/video/fbdev/fsl-diu-fb.c > @@ -384,6 +384,7 @@ struct fsl_diu_data { > __le16 next_cursor[MAX_CURS * MAX_CURS] __aligned(32); > uint8_t edid_data[EDID_LENGTH]; > bool has_edid; > + bool has_dev_attr; > } __aligned(32); > > /* Determine the DMA address of a member of the fsl_diu_data structure */ > @@ -1809,6 +1810,7 @@ static int fsl_diu_probe(struct platform_device *pdev) > data->dev_attr.attr.name); > } > > + data->has_dev_attr = true; > dev_set_drvdata(&pdev->dev, data); > return 0; > > @@ -1827,6 +1829,10 @@ static void fsl_diu_remove(struct platform_device *pdev) > int i; > > data = dev_get_drvdata(&pdev->dev); > + > + if (data->has_dev_attr) Looking at other drivers (e.g. drivers/net/can/usb/esd_usb.c) it seems that device_remove_file() is ok even if it's not fully initialized... I think you can drop those extra checks. Helge > + device_remove_file(&pdev->dev, &data->dev_attr); > + > disable_lcdc(&data->fsl_diu_info[0]); > > free_irq(data->irq, data->diu_reg);
Yeah, you are right, but I believe it would be better to retain the checks. Anyway, I have submitted the V3 patch which has dropped the checks. Thanks and Regards, Shixiong Ou. 在 2025/3/10 03:42, Helge Deller 写道: > On 3/9/25 09:16, Shixiong Ou wrote: >> From: Shixiong Ou <oushixiong@kylinos.cn> >> >> Call device_remove_file() when driver remove. >> >> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn> >> --- >> v1->v2: >> add has_sysfs_attrs flag. >> >> drivers/video/fbdev/fsl-diu-fb.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/video/fbdev/fsl-diu-fb.c >> b/drivers/video/fbdev/fsl-diu-fb.c >> index 5ac8201c3533..57f7fe6a4c76 100644 >> --- a/drivers/video/fbdev/fsl-diu-fb.c >> +++ b/drivers/video/fbdev/fsl-diu-fb.c >> @@ -384,6 +384,7 @@ struct fsl_diu_data { >> __le16 next_cursor[MAX_CURS * MAX_CURS] __aligned(32); >> uint8_t edid_data[EDID_LENGTH]; >> bool has_edid; >> + bool has_dev_attr; >> } __aligned(32); >> >> /* Determine the DMA address of a member of the fsl_diu_data >> structure */ >> @@ -1809,6 +1810,7 @@ static int fsl_diu_probe(struct platform_device >> *pdev) >> data->dev_attr.attr.name); >> } >> >> + data->has_dev_attr = true; >> dev_set_drvdata(&pdev->dev, data); >> return 0; >> >> @@ -1827,6 +1829,10 @@ static void fsl_diu_remove(struct >> platform_device *pdev) >> int i; >> >> data = dev_get_drvdata(&pdev->dev); >> + >> + if (data->has_dev_attr) > > Looking at other drivers (e.g. drivers/net/can/usb/esd_usb.c) it seems > that device_remove_file() is ok even if it's not fully initialized... > > I think you can drop those extra checks. > > Helge > > >> + device_remove_file(&pdev->dev, &data->dev_attr); >> + >> disable_lcdc(&data->fsl_diu_info[0]); >> >> free_irq(data->irq, data->diu_reg);
diff --git a/drivers/video/fbdev/fsl-diu-fb.c b/drivers/video/fbdev/fsl-diu-fb.c index 5ac8201c3533..57f7fe6a4c76 100644 --- a/drivers/video/fbdev/fsl-diu-fb.c +++ b/drivers/video/fbdev/fsl-diu-fb.c @@ -384,6 +384,7 @@ struct fsl_diu_data { __le16 next_cursor[MAX_CURS * MAX_CURS] __aligned(32); uint8_t edid_data[EDID_LENGTH]; bool has_edid; + bool has_dev_attr; } __aligned(32); /* Determine the DMA address of a member of the fsl_diu_data structure */ @@ -1809,6 +1810,7 @@ static int fsl_diu_probe(struct platform_device *pdev) data->dev_attr.attr.name); } + data->has_dev_attr = true; dev_set_drvdata(&pdev->dev, data); return 0; @@ -1827,6 +1829,10 @@ static void fsl_diu_remove(struct platform_device *pdev) int i; data = dev_get_drvdata(&pdev->dev); + + if (data->has_dev_attr) + device_remove_file(&pdev->dev, &data->dev_attr); + disable_lcdc(&data->fsl_diu_info[0]); free_irq(data->irq, data->diu_reg);