| Message ID | 20230310095301.1596369-1-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | platform/x86/amd: pmc: decouple CONFIG_SUSPEND from AMD STB API usage | expand |
Hi Hans, On 3/10/2023 3:23 PM, Shyam Sundar S K wrote: > The initial introduction of AMD STB was limited to only suspend/resume > use cases, but the need for STB APIs like amd_pmc_write_stb() have grown > overtime and guarding it with CONFIG_SUSPEND seems to be incorrect. > > Also, with the recent commit, it seems to create a build failure where > CONFIG_SUSPEND is not enabled. Remove the association of CONFIG_SUSPEND > with STB APIs altogether. > > Cc: Sanket Goswami <Sanket.Goswami@amd.com> > Fixes: b0d4bb973539 ("platform/x86/amd: pmc: Write dummy postcode into the STB DRAM") > Reported-by: Robert Swiecki <robert@swiecki.net> > Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t > Reported-by: Steven Rostedt <rostedt@goodmis.org> > Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t I had to use the same "Link" tag twice as Robert & Steven reported the same issue and without that checkpatch fails. Hope that's fine. Thanks, Shyam > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/platform/x86/amd/pmc.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c > index ab05b9ee6655..8db608e2c1ea 100644 > --- a/drivers/platform/x86/amd/pmc.c > +++ b/drivers/platform/x86/amd/pmc.c > @@ -171,9 +171,7 @@ MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs"); > static struct amd_pmc_dev pmc; > static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); > static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); > -#ifdef CONFIG_SUSPEND > static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data); > -#endif > > static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset) > { > @@ -905,7 +903,6 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) > return 0; > } > > -#ifdef CONFIG_SUSPEND > static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) > { > int err; > @@ -926,7 +923,6 @@ static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) > > return 0; > } > -#endif > > static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf) > { >
Hi Shyam, On 3/10/23 10:53, Shyam Sundar S K wrote: > The initial introduction of AMD STB was limited to only suspend/resume > use cases, but the need for STB APIs like amd_pmc_write_stb() have grown > overtime and guarding it with CONFIG_SUSPEND seems to be incorrect. > > Also, with the recent commit, it seems to create a build failure where > CONFIG_SUSPEND is not enabled. Remove the association of CONFIG_SUSPEND > with STB APIs altogether. > > Cc: Sanket Goswami <Sanket.Goswami@amd.com> > Fixes: b0d4bb973539 ("platform/x86/amd: pmc: Write dummy postcode into the STB DRAM") > Reported-by: Robert Swiecki <robert@swiecki.net> > Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t > Reported-by: Steven Rostedt <rostedt@goodmis.org> > Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> This is already fixed (in a nicer way) by: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=24efcdf03d85bb73df0ba99f69c8d238e7ada0e5 Regards, Hans > --- > drivers/platform/x86/amd/pmc.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c > index ab05b9ee6655..8db608e2c1ea 100644 > --- a/drivers/platform/x86/amd/pmc.c > +++ b/drivers/platform/x86/amd/pmc.c > @@ -171,9 +171,7 @@ MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs"); > static struct amd_pmc_dev pmc; > static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); > static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); > -#ifdef CONFIG_SUSPEND > static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data); > -#endif > > static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset) > { > @@ -905,7 +903,6 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) > return 0; > } > > -#ifdef CONFIG_SUSPEND > static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) > { > int err; > @@ -926,7 +923,6 @@ static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) > > return 0; > } > -#endif > > static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf) > {
On 3/10/2023 3:57 PM, Hans de Goede wrote: > Hi Shyam, > > On 3/10/23 10:53, Shyam Sundar S K wrote: >> The initial introduction of AMD STB was limited to only suspend/resume >> use cases, but the need for STB APIs like amd_pmc_write_stb() have grown >> overtime and guarding it with CONFIG_SUSPEND seems to be incorrect. >> >> Also, with the recent commit, it seems to create a build failure where >> CONFIG_SUSPEND is not enabled. Remove the association of CONFIG_SUSPEND >> with STB APIs altogether. >> >> Cc: Sanket Goswami <Sanket.Goswami@amd.com> >> Fixes: b0d4bb973539 ("platform/x86/amd: pmc: Write dummy postcode into the STB DRAM") >> Reported-by: Robert Swiecki <robert@swiecki.net> >> Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t >> Reported-by: Steven Rostedt <rostedt@goodmis.org> >> Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > > This is already fixed (in a nicer way) by: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=24efcdf03d85bb73df0ba99f69c8d238e7ada0e5 Thanks Hans. I missed this.. Thanks, Shyam > > Regards, > > Hans > > > > >> --- >> drivers/platform/x86/amd/pmc.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c >> index ab05b9ee6655..8db608e2c1ea 100644 >> --- a/drivers/platform/x86/amd/pmc.c >> +++ b/drivers/platform/x86/amd/pmc.c >> @@ -171,9 +171,7 @@ MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs"); >> static struct amd_pmc_dev pmc; >> static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); >> static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); >> -#ifdef CONFIG_SUSPEND >> static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data); >> -#endif >> >> static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset) >> { >> @@ -905,7 +903,6 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) >> return 0; >> } >> >> -#ifdef CONFIG_SUSPEND >> static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) >> { >> int err; >> @@ -926,7 +923,6 @@ static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) >> >> return 0; >> } >> -#endif >> >> static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf) >> { >
diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c index ab05b9ee6655..8db608e2c1ea 100644 --- a/drivers/platform/x86/amd/pmc.c +++ b/drivers/platform/x86/amd/pmc.c @@ -171,9 +171,7 @@ MODULE_PARM_DESC(disable_workarounds, "Disable workarounds for platform bugs"); static struct amd_pmc_dev pmc; static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); -#ifdef CONFIG_SUSPEND static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data); -#endif static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset) { @@ -905,7 +903,6 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) return 0; } -#ifdef CONFIG_SUSPEND static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) { int err; @@ -926,7 +923,6 @@ static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data) return 0; } -#endif static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf) {
The initial introduction of AMD STB was limited to only suspend/resume use cases, but the need for STB APIs like amd_pmc_write_stb() have grown overtime and guarding it with CONFIG_SUSPEND seems to be incorrect. Also, with the recent commit, it seems to create a build failure where CONFIG_SUSPEND is not enabled. Remove the association of CONFIG_SUSPEND with STB APIs altogether. Cc: Sanket Goswami <Sanket.Goswami@amd.com> Fixes: b0d4bb973539 ("platform/x86/amd: pmc: Write dummy postcode into the STB DRAM") Reported-by: Robert Swiecki <robert@swiecki.net> Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t Reported-by: Steven Rostedt <rostedt@goodmis.org> Link: https://lore.kernel.org/platform-driver-x86/20230309225016.2e8cd211@gandalf.local.home/T/#t Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> --- drivers/platform/x86/amd/pmc.c | 4 ---- 1 file changed, 4 deletions(-)