On 9/2/2022 04:40, Szuying Chen wrote:
From: Szuying Chen <Chloe_Chen@xxxxxxxxxxxxxx> The patch add ASMedia NVM formats. Signed-off-by: Szuying Chen <Chloe_Chen@xxxxxxxxxxxxxx> --- 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