On Thu, 16 Nov 2023, Shyam Sundar S K wrote: > After talking to the PMFW team, its understood that the "get dram size" > mbox command would only be supported on specific platforms (like Mendocino) > and not all. So, adjust getting DRAM size behavior such that, > > - if that's Rembrandt or Mendocino and the underlying PMFW knows how > to execute the "get dram size" command it shall give the custom dram size. > > - if the underlying FW does not report the dram size, we just proceed > further and assign the default dram size. Hi, This still talks only about the solution but I gave you a specific instruction to include the description of the problem that warrants Fixes tag. :-( So please include something along the lines you had in v1 to describe how the current code fails because the version numbers are not initialized. > Cc: Mark Hasemeyer <markhas@xxxxxxxxxxxx> Promote Mark to Reported-by. Codewise, this is seems fine and is IMO nice simplification. -- i. > Link: https://lore.kernel.org/platform-driver-x86/3b224c62-a1d8-41bd-aced-5825f5f20e66@xxxxxxx/ > Fixes: be8325fb3d8c ("platform/x86/amd: pmc: Get STB DRAM size from PMFW") > Suggested-by: Sanket Goswami <Sanket.Goswami@xxxxxxx> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> > --- > v3: > - Based on review-ilpo branch > - Remove amd_pmc_get_dram_size() function > - Remove prints that are noisy > > v2: > - Based on review-ilpo branch > - Drop calling get smu version from probe. > > drivers/platform/x86/amd/pmc/pmc.c | 31 ++---------------------------- > 1 file changed, 2 insertions(+), 29 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c > index cd6ac04c1468..c3104714b480 100644 > --- a/drivers/platform/x86/amd/pmc/pmc.c > +++ b/drivers/platform/x86/amd/pmc/pmc.c > @@ -964,33 +964,6 @@ static const struct pci_device_id pmc_pci_ids[] = { > { } > }; > > -static int amd_pmc_get_dram_size(struct amd_pmc_dev *dev) > -{ > - int ret; > - > - switch (dev->cpu_id) { > - case AMD_CPU_ID_YC: > - if (!(dev->major > 90 || (dev->major == 90 && dev->minor > 39))) { > - ret = -EINVAL; > - goto err_dram_size; > - } > - break; > - default: > - ret = -EINVAL; > - goto err_dram_size; > - } > - > - ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true); > - if (ret || !dev->dram_size) > - goto err_dram_size; > - > - return 0; > - > -err_dram_size: > - dev_err(dev->dev, "DRAM size command not supported for this platform\n"); > - return ret; > -} > - > static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) > { > u32 phys_addr_low, phys_addr_hi; > @@ -1009,8 +982,8 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) > return -EIO; > > /* Get DRAM size */ > - ret = amd_pmc_get_dram_size(dev); > - if (ret) > + ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, true); > + if (ret || !dev->dram_size) > dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX; > > /* Get STB DRAM address */ >
