diff mbox series

[1/2] driver core:add device's platform_data set for faux device

Message ID b03b374bc3adad275893e2ad60d4bf5a0ad358e3.1746662386.git.zhouzongmin@kylinos.cn
State New
Headers show
Series some changes based on faux bus | expand

Commit Message

Zongmin Zhou May 8, 2025, 9:11 a.m. UTC
From: Zongmin Zhou <zhouzongmin@kylinos.cn>

Most drivers based on platform bus may have specific data
for the device.And will get this specific data to use
after device added.
So keep the setting for device's platform_data is necessary
for converting platform device to faux device.

Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 drivers/base/faux.c                | 9 +++++++--
 drivers/char/tlclk.c               | 2 +-
 drivers/misc/lis3lv02d/lis3lv02d.c | 2 +-
 include/linux/device/faux.h        | 3 ++-
 4 files changed, 11 insertions(+), 5 deletions(-)

Comments

Greg Kroah-Hartman May 8, 2025, 9:45 a.m. UTC | #1
On Thu, May 08, 2025 at 05:11:47PM +0800, Zongmin Zhou wrote:
> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> 
> Most drivers based on platform bus may have specific data
> for the device.And will get this specific data to use
> after device added.
> So keep the setting for device's platform_data is necessary
> for converting platform device to faux device.

I do not understand, why not just use the platform_data field directly
in the faux device structure?  Why change all callers to now have to
keep track of an additional pointer in these create functions?  That
just adds complexity for everyone when almost no one will need it.

thanks,

greg k-h
Zongmin Zhou May 9, 2025, 2:41 a.m. UTC | #2
On 2025/5/8 17:45, Greg KH wrote:
> On Thu, May 08, 2025 at 05:11:47PM +0800, Zongmin Zhou wrote:
>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>
>> Most drivers based on platform bus may have specific data
>> for the device.And will get this specific data to use
>> after device added.
>> So keep the setting for device's platform_data is necessary
>> for converting platform device to faux device.
> I do not understand, why not just use the platform_data field directly
> in the faux device structure?  Why change all callers to now have to
> keep track of an additional pointer in these create functions?  That
> just adds complexity for everyone when almost no one will need it.
In fact, I have tried other approaches.
However, I found that it must be set after creating faux_dev and before 
calling the device_add() function.

Because the execution of the driver init and the device probe function 
is asynchronous,
and the actual test shows that the probe function is executed
before faux_device_create_with_groups () returns faux_device for the caller.
But the probe and related functions may need to get plat_data.If 
plat_data is set after
faux_device_create_with_groups() is completed and fdev is returned, the 
probe function will get NULL.

Take vhci-hcd as an example:
vhci_hcd_init() calls faux_device_create_with_groups(),
Once device_add() is called, vhci_hcd_probe() will be executed immediately.
Therefore, the probe function will attempt to obtain plat_data
before vhci_hcd_init() receives the return value of faux_device.
It's too late to set plat_data after get the return value of faux_device.

If there is anything not clearly or other good ways to handle this, 
please let me know.

Thanks very much.

>
> thanks,
>
> greg k-h
Greg Kroah-Hartman May 21, 2025, 10:51 a.m. UTC | #3
On Fri, May 09, 2025 at 10:41:11AM +0800, Zongmin Zhou wrote:
> 
> On 2025/5/8 17:45, Greg KH wrote:
> > On Thu, May 08, 2025 at 05:11:47PM +0800, Zongmin Zhou wrote:
> > > From: Zongmin Zhou <zhouzongmin@kylinos.cn>
> > > 
> > > Most drivers based on platform bus may have specific data
> > > for the device.And will get this specific data to use
> > > after device added.
> > > So keep the setting for device's platform_data is necessary
> > > for converting platform device to faux device.
> > I do not understand, why not just use the platform_data field directly
> > in the faux device structure?  Why change all callers to now have to
> > keep track of an additional pointer in these create functions?  That
> > just adds complexity for everyone when almost no one will need it.
> In fact, I have tried other approaches.
> However, I found that it must be set after creating faux_dev and before
> calling the device_add() function.
> 
> Because the execution of the driver init and the device probe function is
> asynchronous,
> and the actual test shows that the probe function is executed
> before faux_device_create_with_groups () returns faux_device for the caller.
> But the probe and related functions may need to get plat_data.If plat_data
> is set after
> faux_device_create_with_groups() is completed and fdev is returned, the
> probe function will get NULL.
> 
> Take vhci-hcd as an example:
> vhci_hcd_init() calls faux_device_create_with_groups(),
> Once device_add() is called, vhci_hcd_probe() will be executed immediately.
> Therefore, the probe function will attempt to obtain plat_data
> before vhci_hcd_init() receives the return value of faux_device.
> It's too late to set plat_data after get the return value of faux_device.
> 
> If there is anything not clearly or other good ways to handle this, please
> let me know.

I think you need to unwind the "probe" logic here as it's not needed at
all.  After you create the faux device, then continue on with the logic
that is currently in the probe callback.  No need to split this out at
all, it's the same device being used/handled here, just unwind the logic
a bit and you should be ok.

hope this helps,

greg k-h
Zongmin Zhou May 28, 2025, 9:21 a.m. UTC | #4
On 2025/5/21 18:51, Greg KH wrote:
> On Fri, May 09, 2025 at 10:41:11AM +0800, Zongmin Zhou wrote:
>> On 2025/5/8 17:45, Greg KH wrote:
>>> On Thu, May 08, 2025 at 05:11:47PM +0800, Zongmin Zhou wrote:
>>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>>
>>>> Most drivers based on platform bus may have specific data
>>>> for the device.And will get this specific data to use
>>>> after device added.
>>>> So keep the setting for device's platform_data is necessary
>>>> for converting platform device to faux device.
>>> I do not understand, why not just use the platform_data field directly
>>> in the faux device structure?  Why change all callers to now have to
>>> keep track of an additional pointer in these create functions?  That
>>> just adds complexity for everyone when almost no one will need it.
>> In fact, I have tried other approaches.
>> However, I found that it must be set after creating faux_dev and before
>> calling the device_add() function.
>>
>> Because the execution of the driver init and the device probe function is
>> asynchronous,
>> and the actual test shows that the probe function is executed
>> before faux_device_create_with_groups () returns faux_device for the caller.
>> But the probe and related functions may need to get plat_data.If plat_data
>> is set after
>> faux_device_create_with_groups() is completed and fdev is returned, the
>> probe function will get NULL.
>>
>> Take vhci-hcd as an example:
>> vhci_hcd_init() calls faux_device_create_with_groups(),
>> Once device_add() is called, vhci_hcd_probe() will be executed immediately.
>> Therefore, the probe function will attempt to obtain plat_data
>> before vhci_hcd_init() receives the return value of faux_device.
>> It's too late to set plat_data after get the return value of faux_device.
>>
>> If there is anything not clearly or other good ways to handle this, please
>> let me know.
> I think you need to unwind the "probe" logic here as it's not needed at
> all.  After you create the faux device, then continue on with the logic
> that is currently in the probe callback.  No need to split this out at
> all, it's the same device being used/handled here, just unwind the logic
> a bit and you should be ok.
I'm very sorry for the late reply.

Yes,actually vhci_hcd_init() can call probe directly without
by the faux_probe call it automatically.
I can make this change for vhci-hcd driver.

Thanks for your suggestion.
>
> hope this helps,
>
> greg k-h
diff mbox series

Patch

diff --git a/drivers/base/faux.c b/drivers/base/faux.c
index 407c1d1aad50..42ec843235cb 100644
--- a/drivers/base/faux.c
+++ b/drivers/base/faux.c
@@ -94,6 +94,8 @@  static void faux_device_release(struct device *dev)
  *		into, can be NULL.
  * @groups:	The set of sysfs attributes that will be created for this
  *		device when it is registered with the driver core.
+ * @plat_data:	The specific data that want to associated with this new device,
+ * 		can be NULL.Just call 'dev_get_platdata()' to get the data.
  *
  * Create a new faux device and register it in the driver core properly.
  * If present, callbacks in @faux_ops will be called with the device that
@@ -113,7 +115,8 @@  static void faux_device_release(struct device *dev)
 struct faux_device *faux_device_create_with_groups(const char *name,
 						   struct device *parent,
 						   const struct faux_device_ops *faux_ops,
-						   const struct attribute_group **groups)
+						   const struct attribute_group **groups,
+						   void *plat_data)
 {
 	struct faux_object *faux_obj;
 	struct faux_device *faux_dev;
@@ -141,6 +144,8 @@  struct faux_device *faux_device_create_with_groups(const char *name,
 	dev->groups = groups;
 	dev_set_name(dev, "%s", name);
 
+	dev->platform_data = plat_data;
+
 	ret = device_add(dev);
 	if (ret) {
 		pr_err("%s: device_add for faux device '%s' failed with %d\n",
@@ -191,7 +196,7 @@  struct faux_device *faux_device_create(const char *name,
 				       struct device *parent,
 				       const struct faux_device_ops *faux_ops)
 {
-	return faux_device_create_with_groups(name, parent, faux_ops, NULL);
+	return faux_device_create_with_groups(name, parent, faux_ops, NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(faux_device_create);
 
diff --git a/drivers/char/tlclk.c b/drivers/char/tlclk.c
index b381ea7e85d2..9334880dec67 100644
--- a/drivers/char/tlclk.c
+++ b/drivers/char/tlclk.c
@@ -813,7 +813,7 @@  static int __init tlclk_init(void)
 		goto out3;
 	}
 
-	tlclk_device = faux_device_create_with_groups("telco_clock", NULL, NULL, tlclk_groups);
+	tlclk_device = faux_device_create_with_groups("telco_clock", NULL, NULL, tlclk_groups, NULL);
 	if (!tlclk_device) {
 		ret = -ENODEV;
 		goto out4;
diff --git a/drivers/misc/lis3lv02d/lis3lv02d.c b/drivers/misc/lis3lv02d/lis3lv02d.c
index 6957091ab6de..e63cb353f425 100644
--- a/drivers/misc/lis3lv02d/lis3lv02d.c
+++ b/drivers/misc/lis3lv02d/lis3lv02d.c
@@ -864,7 +864,7 @@  ATTRIBUTE_GROUPS(lis3lv02d);
 
 static int lis3lv02d_add_fs(struct lis3lv02d *lis3)
 {
-	lis3->fdev = faux_device_create_with_groups(DRIVER_NAME, NULL, NULL, lis3lv02d_groups);
+	lis3->fdev = faux_device_create_with_groups(DRIVER_NAME, NULL, NULL, lis3lv02d_groups, NULL);
 	if (!lis3->fdev)
 		return -ENODEV;
 
diff --git a/include/linux/device/faux.h b/include/linux/device/faux.h
index 9f43c0e46aa4..36532288a09a 100644
--- a/include/linux/device/faux.h
+++ b/include/linux/device/faux.h
@@ -53,7 +53,8 @@  struct faux_device *faux_device_create(const char *name,
 struct faux_device *faux_device_create_with_groups(const char *name,
 						   struct device *parent,
 						   const struct faux_device_ops *faux_ops,
-						   const struct attribute_group **groups);
+						   const struct attribute_group **groups,
+						   void *plat_data);
 void faux_device_destroy(struct faux_device *faux_dev);
 
 static inline void *faux_device_get_drvdata(const struct faux_device *faux_dev)