mbox series

[v7,0/3] thunderbolt: add vendor's NVM formats

Message ID 20220829111059.665305-1-chensiying21@gmail.com
Headers show
Series thunderbolt: add vendor's NVM formats | expand

Message

Szuying Chen Aug. 29, 2022, 11:10 a.m. UTC 
From: Szuying Chen <Chloe_Chen@asmedia.com.tw>

The patch series for vendors to extend their NVM format.

Szuying Chen (3):
  thunderbolt: Add vendor's specific operations of NVM
  thunderbolt: Modify tb_nvm major and minor size.
  thunderbolt: To extend ASMedia NVM formats.

 drivers/thunderbolt/nvm.c    | 274 +++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/switch.c | 105 +++-----------
 drivers/thunderbolt/tb.h     |  11 +-
 3 files changed, 303 insertions(+), 87 deletions(-)

--
2.34.1

Comments

Mika Westerberg Aug. 30, 2022, 1:53 p.m. UTC | #1 
On Mon, Aug 29, 2022 at 07:10:59PM +0800, Szuying Chen wrote:
> From: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> 
> The patch add ASMedia NVM formats. And add tb_switch_nvm_upgradeable()
> to enable firmware upgrade.
> 
> Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> ---
> Add ASMedia NVM formats. And fix asmedia_nvm_version() part of code so
> that easier to read.
> 
>  drivers/thunderbolt/nvm.c    | 68 ++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/switch.c |  3 ++
>  drivers/thunderbolt/tb.h     |  1 +
>  3 files changed, 72 insertions(+)
> 
> diff --git a/drivers/thunderbolt/nvm.c b/drivers/thunderbolt/nvm.c
> index 91c8848b4d2e..c69db5b65f7d 100644
> --- a/drivers/thunderbolt/nvm.c
> +++ b/drivers/thunderbolt/nvm.c
> @@ -15,16 +15,25 @@
>  /* Switch NVM support */
>  #define NVM_CSS		0x10
> 
> +/* Vendor ID of the Router. It's assigned by the USB-IF */

Pretty useless comment.

> +#define ROUTER_VENDOR_ID_ASMEDIA 0x174c
> +
> +/* ASMedia specific NVM offsets */
> +#define ASMEDIA_NVM_DATE	0x1c
> +#define ASMEDIA_NVM_VERSION	0x28
> +
>  static DEFINE_IDA(nvm_ida);
> 
>  /**
>   * struct tb_nvm_vendor_ops - Vendor NVM specific operations
>   * @read_version: Used NVM read get Firmware version.
>   * @validate: Vendors have their validate method before NVM write.
> + * @nvm_upgrade: Enable NVM upgrade.
>   */
>  struct tb_nvm_vendor_ops {
>  	int (*read_version)(struct tb_switch *sw);
>  	int (*validate)(struct tb_switch *sw);
> +	void (*nvm_upgrade)(struct tb_switch *sw);
>  };
> 
>  static inline int nvm_read(struct tb_switch *sw, unsigned int address,
> @@ -128,11 +137,49 @@ static int intel_nvm_validate(struct tb_switch *sw)
>  	return 0;
>  }
> 
> +static int asmedia_nvm_version(struct tb_switch *sw)
> +{
> +	struct tb_nvm *nvm = sw->nvm;
> +	u32 val;
> +	int ret;
> +
> +	/* ASMedia get version and date format is xxxxxx.xxxxxx */
> +	ret = nvm_read(sw, ASMEDIA_NVM_VERSION, &val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	nvm->major = (((u8)val) << 0x10 | ((u8)(val >> 0x8)) << 0x8 | (u8)(val >> 0x10));
> +
> +	ret = nvm_read(sw, ASMEDIA_NVM_DATE, &val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	nvm->minor = (((u8)val) << 0x10 | ((u8)(val >> 0x8)) << 0x8 | (u8)(val >> 0x10));
> +
> +	/*
> +	 * Asmedia NVM size fixed on 512K. We currently have no plan
> +	 * to increase size in the future.
> +	 */
> +	nvm->nvm_size = SZ_512K;
> +
> +	return 0;
> +}
> +
> +static void tb_switch_set_nvm_upgrade(struct tb_switch *sw)
> +{
> +	sw->no_nvm_upgrade = false;
> +}
> +
>  static const struct tb_nvm_vendor_ops intel_switch_nvm_ops = {
>  	.read_version = intel_nvm_version,
>  	.validate = intel_nvm_validate,
>  };
> 
> +static const struct tb_nvm_vendor_ops asmedia_switch_nvm_ops = {
> +	.nvm_upgrade = tb_switch_set_nvm_upgrade,

This is bad name IMHO. It does not really upragade the NVM so perhaps
something like .nvm_upgradeable?

Hower, I don't think this is needed at all because instead of the hack
in tb_start():

	tb->root_switch->no_nvm_upgrade = true;

we should make this:

	tb->root_switch->no_nvm_upgrade = !tb_switch_is_usb4(sw);

and then it only depends on the fact whether the router implements the
necessary NVM operations (please double check if this actually works).

> +	.read_version = asmedia_nvm_version,
> +};
> +
>  struct switch_nvm_vendor {
>  	u16 vendor;
>  	const struct tb_nvm_vendor_ops *vops;
> @@ -143,6 +190,27 @@ static const struct switch_nvm_vendor switch_nvm_vendors[] = {
>  	{ 0x8087, &intel_switch_nvm_ops },
>  };
> 
> +/**
> + * tb_switch_nvm_upgradeable() - Enable NVM upgrade of a switch
> + * @sw: Switch whose NVM upgrade to enable
> + *
> + * This function must be called before creating the switch devices, it will
> + * make the no_active NVM device visible.

non_active

> + */
> +void tb_switch_nvm_upgradeable(struct tb_switch *sw)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(switch_nvm_vendors); i++) {
> +		const struct switch_nvm_vendor *v = &switch_nvm_vendors[i];
> +
> +		if (v->vendor == sw->config.vendor_id) {
> +			if (v->vops->nvm_upgrade)
> +				v->vops->nvm_upgrade(sw);
> +		}
> +	}
> +}
> +
>  /**
>   * tb_switch_nvm_validate() - Validate NVM image
>   * @switch: Switch to NVM write
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 2dbfd75202bf..f8dc18f6c5c8 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -2822,6 +2822,9 @@ int tb_switch_add(struct tb_switch *sw)
>  			return ret;
>  	}
> 
> +	/* Enable the NVM firmware upgrade */
> +	tb_switch_nvm_upgradeable(sw);
> +
>  	ret = device_add(&sw->dev);
>  	if (ret) {
>  		dev_err(&sw->dev, "failed to add device: %d\n", ret);
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 9cf62d5f25d2..642af7473851 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -773,6 +773,7 @@ int tb_switch_reset(struct tb_switch *sw);
>  int tb_switch_wait_for_bit(struct tb_switch *sw, u32 offset, u32 bit,
>  			   u32 value, int timeout_msec);
>  int tb_switch_nvm_validate(struct tb_switch *sw);
> +void tb_switch_nvm_upgradeable(struct tb_switch *sw);
>  void tb_sw_set_unplugged(struct tb_switch *sw);
>  struct tb_port *tb_switch_find_port(struct tb_switch *sw,
> 				    enum tb_port_type type);
> --
> 2.34.1
Mika Westerberg Aug. 30, 2022, 1:59 p.m. UTC | #2 
Hi,

On Mon, Aug 29, 2022 at 07:10:56PM +0800, Szuying Chen wrote:
> From: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> 
> The patch series for vendors to extend their NVM format.

Starts looking better now :) I sent a couple of comments in separate
emails please address those and also the comments you got from Greg and
Mario.

You can put the changelog here in the next iteration of the series.

> Szuying Chen (3):
>   thunderbolt: Add vendor's specific operations of NVM
>   thunderbolt: Modify tb_nvm major and minor size.
>   thunderbolt: To extend ASMedia NVM formats.
> 
>  drivers/thunderbolt/nvm.c    | 274 +++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/switch.c | 105 +++-----------
>  drivers/thunderbolt/tb.h     |  11 +-
>  3 files changed, 303 insertions(+), 87 deletions(-)
> 
> --
> 2.34.1