Hi Simon,
On Tue, Jan 22, 2019 at 9:14 AM Simon Glass <s...@chromium.org> wrote:
>
> At present the PCH has 4 operations and these are reasonably widely used
> in the drivers. But sometimes we want to add rarely used operations, and
> each of these currently adds to the size of the PCH operations table.
>
> Add an ioctl() method which can be easily expanded without any more impact
> on the operations table.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> drivers/pch/pch-uclass.c | 10 ++++++++
> drivers/pch/sandbox_pch.c | 17 ++++++++++++++
> include/pch.h | 48 ++++++++++++++++++++++++++++++++++++++-
> test/dm/pch.c | 19 ++++++++++++++++
> 4 files changed, 93 insertions(+), 1 deletion(-)
>
Reviewed-by: Bin Meng <bmeng...@gmail.com>
But please see comments below.
> diff --git a/drivers/pch/pch-uclass.c b/drivers/pch/pch-uclass.c
> index 831b283d7b..caf8b72803 100644
> --- a/drivers/pch/pch-uclass.c
> +++ b/drivers/pch/pch-uclass.c
> @@ -51,6 +51,16 @@ int pch_get_io_base(struct udevice *dev, u32 *iobasep)
> return ops->get_io_base(dev, iobasep);
> }
>
> +int pch_ioctl(struct udevice *dev, ulong req, void *data, int size)
> +{
> + struct pch_ops *ops = pch_get_ops(dev);
> +
> + if (!ops->ioctl)
> + return -ENOSYS;
> +
> + return ops->ioctl(dev, req, data, size);
> +}
> +
> UCLASS_DRIVER(pch) = {
> .id = UCLASS_PCH,
> .name = "pch",
> diff --git a/drivers/pch/sandbox_pch.c b/drivers/pch/sandbox_pch.c
> index 2209bff8d4..43688368ed 100644
> --- a/drivers/pch/sandbox_pch.c
> +++ b/drivers/pch/sandbox_pch.c
> @@ -48,11 +48,28 @@ static int sandbox_pch_get_io_base(struct udevice *dev,
> u32 *iobasep)
> return 0;
> }
>
> +int sandbox_pch_ioctl(struct udevice *dev, enum pch_req_t req, void *data,
> + int size)
> +{
> + switch (req) {
> + case PCH_REQ_TEST1:
> + return -ENOSYS;
> + case PCH_REQ_TEST2:
> + return *(char *)data;
> + case PCH_REQ_TEST3:
> + *(char *)data = 'x';
> + return 1;
> + default:
> + return -ENOSYS;
> + }
> +}
> +
> static const struct pch_ops sandbox_pch_ops = {
> .get_spi_base = sandbox_pch_get_spi_base,
> .set_spi_protect = sandbox_pch_set_spi_protect,
> .get_gpio_base = sandbox_pch_get_gpio_base,
> .get_io_base = sandbox_pch_get_io_base,
> + .ioctl = sandbox_pch_ioctl,
> };
>
> static const struct udevice_id sandbox_pch_ids[] = {
> diff --git a/include/pch.h b/include/pch.h
> index 73994b8343..c9995702c1 100644
> --- a/include/pch.h
> +++ b/include/pch.h
> @@ -11,7 +11,20 @@
>
> #define BIOS_CTRL_BIOSWE BIT(0)
>
> -/* Operations for the Platform Controller Hub */
> +/* All the supported PCH ioctls */
> +enum pch_req_t {
> + PCH_REQ_TEST1, /* Test requests for sandbox driver */
> + PCH_REQ_TEST2,
> + PCH_REQ_TEST3,
> +
> + PCH_REQ_COUNT, /* Number of ioctrls supported */
> +};
> +
> +/**
> + * struct pch_ops - Operations for the Platform Controller Hub
> + *
> + * Consider using ioctl() to add rarely used or driver-specific operations.
> + */
> struct pch_ops {
> /**
> * get_spi_base() - get the address of SPI base
> @@ -49,6 +62,23 @@ struct pch_ops {
> * @return 0 if OK, -ve on error (e.g. there is no IO base)
> */
> int (*get_io_base)(struct udevice *dev, u32 *iobasep);
> +
> + /**
> + * ioctl() - perform misc read/write operations
> + *
> + * This is a catch-all operation intended to avoid adding lots of
> + * methods to this uclass, of which few are commonly used. Uncommon
> + * operations that pertain only to a few devices in this uclass should
> + * use this method instead of adding new methods.
> + *
> + * @dev: PCH device to check
> + * @req: PCH request ID
> + * @data: Input/output data
> + * @size: Size of input data (and maximum size of output data)
> + * @return size of output data or 0 on sucesss, -ve on error
From the sandbox ioctl implementation, it never returns 0, so this is
incorrect comment.
> + */
> + int (*ioctl)(struct udevice *dev, enum pch_req_t req, void *data,
> + int size);
> };
>
> #define pch_get_ops(dev) ((struct pch_ops *)(dev)->driver->ops)
> @@ -90,4 +120,20 @@ int pch_get_gpio_base(struct udevice *dev, u32 *gbasep);
> */
> int pch_get_io_base(struct udevice *dev, u32 *iobasep);
>
> +/**
> + * pch_ioctl() - perform misc read/write operations
> + *
> + * This is a catch-all operation intended to avoid adding lots of
> + * methods to this uclass, of which few are commonly used. Uncommon
> + * operations that pertain only to a few devices in this uclass should
> + * use this method instead of adding new methods.
> + *
> + * @dev: PCH device to check
> + * @req: PCH request ID
> + * @data: Input/output data
> + * @size: Size of input data (and maximum size of output data)
> + * @return size of output data or 0 on sucesss, -ve on error
Ditto.
> + */
> +int pch_ioctl(struct udevice *dev, ulong req, void *data, int size);
> +
> #endif
[snip]
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
