All of lore.kernel.org
 help / color / mirror / Atom feed
From: alex.williamson@redhat.com (Alex Williamson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/4] VFIO: platform: add reset struct and lookup table
Date: Tue, 09 Jun 2015 12:26:14 -0600	[thread overview]
Message-ID: <1433874374.4927.114.camel@redhat.com> (raw)
In-Reply-To: <1433516792-16397-2-git-send-email-eric.auger@linaro.org>

On Fri, 2015-06-05 at 17:06 +0200, Eric Auger wrote:
> This patch introduces the vfio_platform_reset_combo struct that
> stores all the information useful to handle the reset modality:
> compat string, name of the reset function, name of the module that
> implements the reset function. A lookup table of such structures
> is added, currently containing a single sentinel element. A new
> type field is added in vfio_platform_device to store what kind of
> reset is associated to the device, if any.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  drivers/vfio/platform/vfio_platform_common.c  |  6 ++++++
>  drivers/vfio/platform/vfio_platform_private.h | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index abcff7a..d970776 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -25,6 +25,12 @@
>  
>  static DEFINE_MUTEX(driver_lock);
>  
> +static const struct vfio_platform_reset_combo reset_lookup_table[] = {
> +	{
> +		.type = VFIO_PLATFORM_RESET_TYPE_MAX
> +	},
> +};
> +
>  static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
>  {
>  	int cnt = 0, i;
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 5d31e04..d864124 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -49,6 +49,10 @@ struct vfio_platform_region {
>  	void __iomem		*ioaddr;
>  };
>  
> +enum vfio_platform_reset_type {
> +	VFIO_PLATFORM_RESET_TYPE_MAX /* last element */,
> +};
> +
>  struct vfio_platform_device {
>  	struct vfio_platform_region	*regions;
>  	u32				num_regions;
> @@ -56,6 +60,7 @@ struct vfio_platform_device {
>  	u32				num_irqs;
>  	int				refcnt;
>  	struct mutex			igate;
> +	enum vfio_platform_reset_type	type;
>  
>  	/*
>  	 * These fields should be filled by the bus specific binder
> @@ -69,6 +74,13 @@ struct vfio_platform_device {
>  	int	(*get_irq)(struct vfio_platform_device *vdev, int i);
>  };
>  
> +struct vfio_platform_reset_combo {
> +	enum vfio_platform_reset_type type;
> +	char *compat;
> +	char *reset_function_name;
> +	char *module_name;
> +};
> +

I don't really understand the benefit of vfio_platform_reset_type, what
does it add?  If we want an array end marker, we could just use NULL,
but if we're dealing with a static table, we could always use ARRAY_SIZE
and avoid an end marker altogether.  If the concern is matching the
symbol to put when the device is released, we could just use
symbol_put_addr() and avoid any sort of lookup.  Seems like there could
also be a smattering of "const" in this struct definition too.

>  extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  				      struct device *dev);
>  extern struct vfio_platform_device *vfio_platform_remove_common

WARNING: multiple messages have this Message-ID (diff)
From: Alex Williamson <alex.williamson@redhat.com>
To: Eric Auger <eric.auger@linaro.org>
Cc: eric.auger@st.com, linux-arm-kernel@lists.infradead.org,
	b.reynal@virtualopensystems.com, christoffer.dall@linaro.org,
	linux-kernel@vger.kernel.org, patches@linaro.org, agraf@suse.de
Subject: Re: [PATCH v2 1/4] VFIO: platform: add reset struct and lookup table
Date: Tue, 09 Jun 2015 12:26:14 -0600	[thread overview]
Message-ID: <1433874374.4927.114.camel@redhat.com> (raw)
In-Reply-To: <1433516792-16397-2-git-send-email-eric.auger@linaro.org>

On Fri, 2015-06-05 at 17:06 +0200, Eric Auger wrote:
> This patch introduces the vfio_platform_reset_combo struct that
> stores all the information useful to handle the reset modality:
> compat string, name of the reset function, name of the module that
> implements the reset function. A lookup table of such structures
> is added, currently containing a single sentinel element. A new
> type field is added in vfio_platform_device to store what kind of
> reset is associated to the device, if any.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>  drivers/vfio/platform/vfio_platform_common.c  |  6 ++++++
>  drivers/vfio/platform/vfio_platform_private.h | 12 ++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index abcff7a..d970776 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -25,6 +25,12 @@
>  
>  static DEFINE_MUTEX(driver_lock);
>  
> +static const struct vfio_platform_reset_combo reset_lookup_table[] = {
> +	{
> +		.type = VFIO_PLATFORM_RESET_TYPE_MAX
> +	},
> +};
> +
>  static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
>  {
>  	int cnt = 0, i;
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 5d31e04..d864124 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -49,6 +49,10 @@ struct vfio_platform_region {
>  	void __iomem		*ioaddr;
>  };
>  
> +enum vfio_platform_reset_type {
> +	VFIO_PLATFORM_RESET_TYPE_MAX /* last element */,
> +};
> +
>  struct vfio_platform_device {
>  	struct vfio_platform_region	*regions;
>  	u32				num_regions;
> @@ -56,6 +60,7 @@ struct vfio_platform_device {
>  	u32				num_irqs;
>  	int				refcnt;
>  	struct mutex			igate;
> +	enum vfio_platform_reset_type	type;
>  
>  	/*
>  	 * These fields should be filled by the bus specific binder
> @@ -69,6 +74,13 @@ struct vfio_platform_device {
>  	int	(*get_irq)(struct vfio_platform_device *vdev, int i);
>  };
>  
> +struct vfio_platform_reset_combo {
> +	enum vfio_platform_reset_type type;
> +	char *compat;
> +	char *reset_function_name;
> +	char *module_name;
> +};
> +

I don't really understand the benefit of vfio_platform_reset_type, what
does it add?  If we want an array end marker, we could just use NULL,
but if we're dealing with a static table, we could always use ARRAY_SIZE
and avoid an end marker altogether.  If the concern is matching the
symbol to put when the device is released, we could just use
symbol_put_addr() and avoid any sort of lookup.  Seems like there could
also be a smattering of "const" in this struct definition too.

>  extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  				      struct device *dev);
>  extern struct vfio_platform_device *vfio_platform_remove_common




  reply	other threads:[~2015-06-09 18:26 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-05 15:06 [PATCH v2 0/4] VFIO platform reset Eric Auger
2015-06-05 15:06 ` Eric Auger
2015-06-05 15:06 ` [PATCH v2 1/4] VFIO: platform: add reset struct and lookup table Eric Auger
2015-06-05 15:06   ` Eric Auger
2015-06-09 18:26   ` Alex Williamson [this message]
2015-06-09 18:26     ` Alex Williamson
2015-06-10 11:45     ` Eric Auger
2015-06-10 11:45       ` Eric Auger
2015-06-05 15:06 ` [PATCH v2 2/4] VFIO: platform: add reset callback Eric Auger
2015-06-05 15:06   ` Eric Auger
2015-06-05 15:06 ` [PATCH v2 3/4] VFIO: platform: populate the reset function on probe Eric Auger
2015-06-05 15:06   ` Eric Auger
2015-06-09 18:26   ` Alex Williamson
2015-06-09 18:26     ` Alex Williamson
2015-06-10 11:44     ` Eric Auger
2015-06-10 11:44       ` Eric Auger
2015-06-10 15:10       ` Alex Williamson
2015-06-10 15:10         ` Alex Williamson
2015-06-10 15:13         ` Eric Auger
2015-06-10 15:13           ` Eric Auger
2015-06-11  9:37         ` Eric Auger
2015-06-11  9:37           ` Eric Auger
2015-06-05 15:06 ` [PATCH v2 4/4] VFIO: platform: Calxeda xgmac reset module Eric Auger
2015-06-05 15:06   ` Eric Auger
2015-06-06 12:57   ` Paul Bolle
2015-06-06 12:57     ` Paul Bolle
2015-06-08  8:02     ` Eric Auger
2015-06-08  8:02       ` Eric Auger
2015-06-05 18:05 ` [PATCH v2 0/4] VFIO platform reset Rob Herring
2015-06-05 18:05   ` Rob Herring
2015-06-05 21:14   ` Scott Wood
2015-06-05 21:14     ` Scott Wood
2015-06-08  7:51     ` Eric Auger
2015-06-08  7:51       ` Eric Auger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1433874374.4927.114.camel@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.