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

From: Szuying Chen <Chloe_Chen@asmedia.com.tw>

The patch series for vendors to extend their NVM format.

v7->v8: The nvm_read() defined in tb.h. Modify
tb_switch_nvm_validate() return value and no_nvm_upgrade bit setting.

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    | 234 +++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/switch.c | 104 +++-------------
 drivers/thunderbolt/tb.c     |   2 +-
 drivers/thunderbolt/tb.h     |  12 +-
 4 files changed, 263 insertions(+), 89 deletions(-)

--
2.34.1

On 9/2/2022 04:40, Szuying Chen wrote:
> From: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> 
> The patch add ASMedia NVM formats.
> 
> Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> ---
> v7->v8: Fix the no_nvm_upgrade bit setting on suggestion by Mika.
> 
>   drivers/thunderbolt/nvm.c | 40 +++++++++++++++++++++++++++++++++++++++
>   drivers/thunderbolt/tb.c  |  2 +-
>   2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/nvm.c b/drivers/thunderbolt/nvm.c
> index 878d705bd0cb..8393d82dd108 100644
> --- a/drivers/thunderbolt/nvm.c
> +++ b/drivers/thunderbolt/nvm.c
> @@ -12,9 +12,16 @@
> 
>   #include "tb.h"
> 
> +/* ID of Router */
> +#define ROUTER_VENDOR_ID_ASMEDIA 0x174c
> +
>   /* Switch NVM support */
>   #define NVM_CSS		0x10
> 
> +/* ASMedia specific NVM offsets */
> +#define ASMEDIA_NVM_DATE	0x1c
> +#define ASMEDIA_NVM_VERSION	0x28
> +
>   static DEFINE_IDA(nvm_ida);
> 
>   /**
> @@ -120,11 +127,43 @@ 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;

Any chance this can also be gleamed from your NVM?  It would future 
proof the kernel code if you did come up with a need to change it in the 
future some day rather than hardcoding.

> +
> +	return 0;
> +}
> +
>   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 = {
> +	.read_version = asmedia_nvm_version,

I recall an earlier version of your patch series was reading the 
customer ID as well.  Would it make sense to have an 
`asmedia_nvm_validate` that checks this matches?

Or any other safety validation that the image is good the kernel might 
want to do?  Checksum or signature or anything?

Even if the hardware does all these verifications it's much easier to 
debug problems if the kernel can do a first line verification to tell 
you what is wrong with the image instead of trying to trace an error 
code from the hardware.

> +};
> +
>   struct switch_nvm_vendor {
>   	u16 vendor;
>   	const struct tb_nvm_vendor_ops *vops;
> @@ -133,6 +172,7 @@ struct switch_nvm_vendor {
>   static const struct switch_nvm_vendor switch_nvm_vendors[] = {
>   	{ PCI_VENDOR_ID_INTEL, &intel_switch_nvm_ops },
>   	{ 0x8087, &intel_switch_nvm_ops },
> +	{ ROUTER_VENDOR_ID_ASMEDIA, &asmedia_switch_nvm_ops },
>   };
> 
>   /**
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 9853f6c7e81d..55faa1b5f815 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -1417,7 +1417,7 @@ static int tb_start(struct tb *tb)
>   	 * mode that is not available so disable firmware upgrade of the
>   	 * root switch.
>   	 */
> -	tb->root_switch->no_nvm_upgrade = true;
> +	tb->root_switch->no_nvm_upgrade = !tb_switch_is_usb4(tb->root_switch);
>   	/* All USB4 routers support runtime PM */
>   	tb->root_switch->rpm = tb_switch_is_usb4(tb->root_switch);
> 
> --
> 2.34.1
>

Hi Szuying Chen,

On Fri, Sep 02, 2022 at 05:40:07PM +0800, Szuying Chen wrote:
> From: Szuying Chen <Chloe_Chen@asmedia.com.tw>
> 
> The patch series for vendors to extend their NVM format.
> 
> v7->v8: The nvm_read() defined in tb.h. Modify
> tb_switch_nvm_validate() return value and no_nvm_upgrade bit setting.
> 
> 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.

Thanks for the patches. There are a couple of things I would still like
to change in this series but I can do those myself, and then I need to
run some testing to make sure Intel NVM formats still work.

No need to send a new version. Once I've done my modifications I will
post the updated series for review and perhaps you can then try on your
side that it still works on ASMedia hardware.

Thanks!