Re: [PATCH v7 1/6] firmware: smccc: Add an Arm SMCCC bus

[Sudeep Holla <sudeep.holla@kernel.org>]

On Mon, Jul 06, 2026 at 08:05:00PM +0530, Aneesh Kumar K.V wrote:
> Sudeep Holla <sudeep.holla@kernel.org> writes:
> 
> > On Thu, Jun 11, 2026 at 06:34:24PM +0530, Aneesh Kumar K.V (Arm) wrote:
> 
> ...
> 
> >> +#define to_arm_smccc_driver(d) \
> >> +	container_of_const(d, struct arm_smccc_driver, driver)
> >> +
> >> +int arm_smccc_driver_register(struct arm_smccc_driver *driver,
> >> +		struct module *owner, const char *mod_name);
> >> +void arm_smccc_driver_unregister(struct arm_smccc_driver *driver);
> >> +struct arm_smccc_device *arm_smccc_device_register(const char *name);
> >> +void arm_smccc_device_unregister(struct arm_smccc_device *smcc_dev);
> >> +
> >
> > I may be overthinking but what will happen if HAVE_ARM_SMCCC_DISCOVERY=n
> > and some driver is compiled using this header ? It should be fine if it
> > fails to compile, just thinking out loud if we need to handle that are not.
> > As long as all the drivers using these depends on HAVE_ARM_SMCCC_DISCOVERY
> > it should be fine I think.
> 
> This will result in a build failure. The driver should either select
> HAVE_ARM_SMCCC_DISCOVERY or depend on HAVE_ARM_SMCCC_DISCOVERY.
> 

That should be fine for now.

> >
> >> +#define arm_smccc_register(driver) \
> >> +	arm_smccc_driver_register(driver, THIS_MODULE, KBUILD_MODNAME)
> >> +#define arm_smccc_unregister(driver) \
> >> +	arm_smccc_driver_unregister(driver)
> >> +
> >> +#define module_arm_smccc_driver(__arm_smccc_driver) \
> >> +	module_driver(__arm_smccc_driver, arm_smccc_register, \
> >> +		      arm_smccc_unregister)
> >> +
> >> +extern const struct bus_type arm_smccc_bus_type;
> >> +
> >> +#endif /* __LINUX_ARM_SMCCC_BUS_H */
> >> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> >> index 23ff24080dfd..c9cee8c5a0b2 100644
> >> --- a/include/linux/mod_devicetable.h
> >> +++ b/include/linux/mod_devicetable.h
> >
> > This file seems to be reworked recently, so you need to rebase it moving
> > smccc specific changes to separate file I think.
> >
> 
> Do we need to add a separate file? I was able to rebase the series on
> 7.2-rc1
> 

Then could be something changed between -rc1 to -rc2, I couldn't apply the
patch.


[...]

> >> +
> >> +/**
> >> + * struct arm_smccc_device_id - Arm SMCCC bus device identifier
> >> + * @name: SMCCC device name
> >> + * @driver_data: driver data
> >> + */
> >> +struct arm_smccc_device_id {
> >> +	char name[ARM_SMCCC_NAME_SIZE];
> >> +	kernel_ulong_t driver_data;
> >
> > Can't find any users of the above driver_data ?
> > Is it for future ? If so, can you add details on how it is supposed to be used
> > if you don't want to drop it.
> >
> 
> That would depend on the driver, wouldn’t it? I was comparing this with
> struct auxiliary_device_id, and very few drivers use driver_data;
> mei_gsc_probe() is one example.
> 

Indeed, but I don't see any attempt to use it currently which is fine but
I also fail how this can be used in future in SMCCC context.

IIUC driver_data is a per-match-entry cookie. It is useful when one driver
supports multiple IDs and wants each ID to select  slightly different
behavior.

The issues as I see are:
1. No driver is using it in this series.
2. The SMCCC bus match code finds the matching table entry, but then
   throws it away. It is not passed to probe for example.

> I am not sure there is a generic rule here. IIUC, it provides a place
> where a driver can add driver-specific data that can be used during the
> probe routine.
>

How is that possible unless we either add a function like

  |  const struct arm_smccc_device_id *
  |  arm_smccc_get_device_id(struct arm_smccc_device *sdev);

to get the pointer to the id_table or that driver_data from the probe.

Or change probe to receive the matched ID/driver_data:

  int (*probe)(struct arm_smccc_device *sdev,
  	     const struct arm_smccc_device_id *id);

Please drop it if you have no plans to use it or see any future usage.

-- 
Regards,
Sudeep