On Thu, Nov 5, 2020 at 2:18 PM Shyam Sundar S K
<Shyam-sundar.S-k@xxxxxxx> wrote:
>
> AMD Power Management Controller driver a.k.a. amd-pmc driver is the
> controller which is meant for the final S2Idle transaction that goes to
> the PMFW running on the AMD SMU (System Management Unit) responsible for
> tuning of the VDD.
>
> Once all the monitored list or the idle constraints are met, this driver
> would go and set the OS_HINT (meaning all the devices have reached to
> their lowest state possible) via the SMU mailboxes.
>
> This driver would also provide some debug capabilities via debugfs.
The rule of thumb when a reviewer gives you a comment against one
place, you have to look for the similar cases in your entire
submission!
...
> +/* SMU Response Codes */
> +#define AMD_PMC_RESULT_CMD_UNKNOWN 0xFE
> +#define AMD_PMC_RESULT_CMD_REJECT_PREREQ 0xFD
> +#define AMD_PMC_RESULT_CMD_REJECT_BUSY 0xFC
> +#define AMD_PMC_RESULT_FAILED 0xFF
> +#define AMD_PMC_RESULT_OK 0x01
I meant sorted by value.
...
> +static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> +{
> +}
> +
Extra blank line.
> +#else
> +static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> +{
> +}
> +
> +static inline void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */
...
> + err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS, AMD_PMC_BASE_ADDR_LO);
> + if (err) {
> + dev_err(dev->dev, "error writing to 0x%x\n", AMD_PMC_SMU_INDEX_ADDRESS);
> + return pcibios_err_to_errno(err);
Bingo!
But... (see also remark at the beginning of this message)
> + }
> +
> + err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, &val);
> + if (err)
> + return err;
..?!
...
> +static struct platform_driver amd_pmc_driver = {
> +};
> +
Extra blank line.
> +module_platform_driver(amd_pmc_driver);
--
With Best Regards,
Andy Shevchenko
