| Message ID | 20231129221225.387952-7-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i3c: master: some improvment for i3c master | expand |
Hi Frank, Frank.Li@nxp.com wrote on Wed, 29 Nov 2023 17:12:25 -0500: > I3C standard 5.1.9.3.15 Get Device Status (GETSTATUS): > Get request for one I3C Target Device to return its current status. > > Add API to fetch it with format1. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/i3c/device.c | 24 ++++++++++++++++++++++++ > drivers/i3c/internals.h | 1 + > drivers/i3c/master.c | 26 ++++++++++++++++++++++++++ > include/linux/i3c/device.h | 1 + > 4 files changed, 52 insertions(+) > > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c > index 1a6a8703dbc3a..aa26cf50ab9c6 100644 > --- a/drivers/i3c/device.c > +++ b/drivers/i3c/device.c > @@ -196,6 +196,30 @@ void i3c_device_free_ibi(struct i3c_device *dev) > } > EXPORT_SYMBOL_GPL(i3c_device_free_ibi); > > +/** > + * i3c_device_getstatus_format1() - Get device status with format 1. > + * @dev: device for which you want to get status. > + * @status: I3C status format 1 > + * > + * Return: 0 in case of success, a negative error core otherwise. > + */ > +int i3c_device_getstatus_format1(struct i3c_device *dev, u16 *status) I'm not a big fan of the opposition between "i3c_device" and "i3c_dev". Could we clarify the prefixes? > +{ > + int ret = -EINVAL; Init not useful > + > + if (!status) > + return -EINVAL; > + > + i3c_bus_normaluse_lock(dev->bus); > + if (dev->desc) > + ret = i3c_dev_getstatus_format1_locked(dev->desc, status); > + > + i3c_bus_normaluse_unlock(dev->bus); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(i3c_device_getstatus_format1); There is no user yet. Maybe this needs to be done in another series? Same below. ... > +int i3c_dev_getstatus_format1_locked(struct i3c_dev_desc *dev, u16 *status) ... Thanks, Miquèl
On Thu, Nov 30, 2023 at 11:19:59AM +0100, Miquel Raynal wrote: > Hi Frank, > > Frank.Li@nxp.com wrote on Wed, 29 Nov 2023 17:12:25 -0500: > > > I3C standard 5.1.9.3.15 Get Device Status (GETSTATUS): > > Get request for one I3C Target Device to return its current status. > > > > Add API to fetch it with format1. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/i3c/device.c | 24 ++++++++++++++++++++++++ > > drivers/i3c/internals.h | 1 + > > drivers/i3c/master.c | 26 ++++++++++++++++++++++++++ > > include/linux/i3c/device.h | 1 + > > 4 files changed, 52 insertions(+) > > > > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c > > index 1a6a8703dbc3a..aa26cf50ab9c6 100644 > > --- a/drivers/i3c/device.c > > +++ b/drivers/i3c/device.c > > @@ -196,6 +196,30 @@ void i3c_device_free_ibi(struct i3c_device *dev) > > } > > EXPORT_SYMBOL_GPL(i3c_device_free_ibi); > > > > +/** > > + * i3c_device_getstatus_format1() - Get device status with format 1. > > + * @dev: device for which you want to get status. > > + * @status: I3C status format 1 > > + * > > + * Return: 0 in case of success, a negative error core otherwise. > > + */ > > +int i3c_device_getstatus_format1(struct i3c_device *dev, u16 *status) > > I'm not a big fan of the opposition between "i3c_device" and "i3c_dev". > Could we clarify the prefixes? look likes: Internal API between device/master/ use i3c_dev External API for I3C target device use i3c_device > > > +{ > > + int ret = -EINVAL; > > Init not useful > > > + > > + if (!status) > > + return -EINVAL; > > + > > + i3c_bus_normaluse_lock(dev->bus); > > + if (dev->desc) > > + ret = i3c_dev_getstatus_format1_locked(dev->desc, status); > > + > > + i3c_bus_normaluse_unlock(dev->bus); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(i3c_device_getstatus_format1); > > There is no user yet. Maybe this needs to be done in another series? > Same below. See https://lore.kernel.org/imx/202311070330.5mylauLR-lkp@intel.com/T/#t > > ... > > > +int i3c_dev_getstatus_format1_locked(struct i3c_dev_desc *dev, u16 *status) > > ... > > Thanks, > Miquèl
Hi Frank, > > > + > > > + if (!status) > > > + return -EINVAL; > > > + > > > + i3c_bus_normaluse_lock(dev->bus); > > > + if (dev->desc) > > > + ret = i3c_dev_getstatus_format1_locked(dev->desc, status); > > > + > > > + i3c_bus_normaluse_unlock(dev->bus); > > > + > > > + return ret; > > > +} > > > +EXPORT_SYMBOL_GPL(i3c_device_getstatus_format1); > > > > There is no user yet. Maybe this needs to be done in another series? > > Same below. > > See > https://lore.kernel.org/imx/202311070330.5mylauLR-lkp@intel.com/T/#t Then this patch should be part of the series you mention. Thanks, Miquèl
On Thu, Nov 30, 2023 at 11:19:59AM +0100, Miquel Raynal wrote: > Hi Frank, > > Frank.Li@nxp.com wrote on Wed, 29 Nov 2023 17:12:25 -0500: > > > I3C standard 5.1.9.3.15 Get Device Status (GETSTATUS): > > Get request for one I3C Target Device to return its current status. > > > > Add API to fetch it with format1. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/i3c/device.c | 24 ++++++++++++++++++++++++ > > drivers/i3c/internals.h | 1 + > > drivers/i3c/master.c | 26 ++++++++++++++++++++++++++ > > include/linux/i3c/device.h | 1 + > > 4 files changed, 52 insertions(+) > > > > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c > > index 1a6a8703dbc3a..aa26cf50ab9c6 100644 > > --- a/drivers/i3c/device.c > > +++ b/drivers/i3c/device.c > > @@ -196,6 +196,30 @@ void i3c_device_free_ibi(struct i3c_device *dev) > > } > > EXPORT_SYMBOL_GPL(i3c_device_free_ibi); > > > > +/** > > + * i3c_device_getstatus_format1() - Get device status with format 1. > > + * @dev: device for which you want to get status. > > + * @status: I3C status format 1 > > + * > > + * Return: 0 in case of success, a negative error core otherwise. > > + */ > > +int i3c_device_getstatus_format1(struct i3c_device *dev, u16 *status) > > I'm not a big fan of the opposition between "i3c_device" and "i3c_dev". > Could we clarify the prefixes? > > > +{ > > + int ret = -EINVAL; > > Init not useful No, if dev->desc is NULL, ret will be random value without init. Frank > > > + > > + if (!status) > > + return -EINVAL; > > + > > + i3c_bus_normaluse_lock(dev->bus); > > + if (dev->desc) > > + ret = i3c_dev_getstatus_format1_locked(dev->desc, status); > > + > > + i3c_bus_normaluse_unlock(dev->bus); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(i3c_device_getstatus_format1); > > There is no user yet. Maybe this needs to be done in another series? > Same below. > > ... > > > +int i3c_dev_getstatus_format1_locked(struct i3c_dev_desc *dev, u16 *status) > > ... > > Thanks, > Miquèl
diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c index 1a6a8703dbc3a..aa26cf50ab9c6 100644 --- a/drivers/i3c/device.c +++ b/drivers/i3c/device.c @@ -196,6 +196,30 @@ void i3c_device_free_ibi(struct i3c_device *dev) } EXPORT_SYMBOL_GPL(i3c_device_free_ibi); +/** + * i3c_device_getstatus_format1() - Get device status with format 1. + * @dev: device for which you want to get status. + * @status: I3C status format 1 + * + * Return: 0 in case of success, a negative error core otherwise. + */ +int i3c_device_getstatus_format1(struct i3c_device *dev, u16 *status) +{ + int ret = -EINVAL; + + if (!status) + return -EINVAL; + + i3c_bus_normaluse_lock(dev->bus); + if (dev->desc) + ret = i3c_dev_getstatus_format1_locked(dev->desc, status); + + i3c_bus_normaluse_unlock(dev->bus); + + return ret; +} +EXPORT_SYMBOL_GPL(i3c_device_getstatus_format1); + /** * i3cdev_to_dev() - Returns the device embedded in @i3cdev * @i3cdev: I3C device diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h index 908a807badaf9..976ad26ca79c2 100644 --- a/drivers/i3c/internals.h +++ b/drivers/i3c/internals.h @@ -24,4 +24,5 @@ int i3c_dev_enable_ibi_locked(struct i3c_dev_desc *dev); int i3c_dev_request_ibi_locked(struct i3c_dev_desc *dev, const struct i3c_ibi_setup *req); void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev); +int i3c_dev_getstatus_format1_locked(struct i3c_dev_desc *dev, u16 *status); #endif /* I3C_INTERNAL_H */ diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index ed5e27cd20811..6a16ebdd180b5 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2924,6 +2924,32 @@ void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev) dev->ibi = NULL; } +int i3c_dev_getstatus_format1_locked(struct i3c_dev_desc *dev, u16 *status) +{ + struct i3c_master_controller *master = i3c_dev_get_master(dev); + struct i3c_ccc_getstatus *format1; + struct i3c_ccc_cmd_dest dest; + struct i3c_ccc_cmd cmd; + int ret; + + format1 = i3c_ccc_cmd_dest_init(&dest, dev->info.dyn_addr, sizeof(*format1)); + if (!format1) + return -ENOMEM; + + i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETSTATUS, &dest, 1); + + ret = i3c_master_send_ccc_cmd_locked(master, &cmd); + if (ret) + goto out; + + *status = be16_to_cpu(format1->status); + +out: + i3c_ccc_cmd_dest_cleanup(&dest); + + return ret; +} + static int __init i3c_init(void) { int res; diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h index ef6217da8253b..5f511bd400f11 100644 --- a/include/linux/i3c/device.h +++ b/include/linux/i3c/device.h @@ -345,4 +345,5 @@ void i3c_device_free_ibi(struct i3c_device *dev); int i3c_device_enable_ibi(struct i3c_device *dev); int i3c_device_disable_ibi(struct i3c_device *dev); +int i3c_device_getstatus_format1(struct i3c_device *dev, u16 *status); #endif /* I3C_DEV_H */
I3C standard 5.1.9.3.15 Get Device Status (GETSTATUS): Get request for one I3C Target Device to return its current status. Add API to fetch it with format1. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/i3c/device.c | 24 ++++++++++++++++++++++++ drivers/i3c/internals.h | 1 + drivers/i3c/master.c | 26 ++++++++++++++++++++++++++ include/linux/i3c/device.h | 1 + 4 files changed, 52 insertions(+)