[PATCH v11 03/14] peci: Add support for PECI bus driver core

Jae Hyun Yoo jae.hyun.yoo at linux.intel.com
Thu Dec 12 11:46:45 AEDT 2019


Hi Andy,

On 12/11/2019 12:18 PM, Andy Shevchenko wrote:
> 
> Nice, we have some drivers under drivers/hwmon. Are they using PECI? How they
> will be integrated to this? Can this be part of drivers/hwmon?

This is designed for SoCs have one-wire PECI hardware which can handle
raw PECI protocol. Some drivers under 'drivers/hwmon' that use
PECI-to-I2C translation hardware are out of scope from this patch.
In case if an SoC supports both PECI-to-I2C translation mode and raw
PECI mode, only raw PECI mode can be integrated to this. NPCM7xx is
the case and peci-npcm driver in this patch set is an example of
the raw PECI driver.

This patch includes peci-cputemp and peci-dimmtemp as raw PECI hwmon
drivers.

>> Changes since v10:
> 
> It's funny I don't remember previous version(s), but anyway I'll comment on
> this later on -- it has at least several style issues / inconveniences.

I CC'ed you in every submissions but you probably forgot that because I
submitted v10 in January this year. Thanks for your review.

>> - Split out peci-dev module from peci-core module.
>> - Added PECI 4.0 command set support.
>> - Refined 32-bit boundary alignment for all PECI ioctl command structs.
>> - Added DMA safe command buffer handling in peci-core.
>> - Refined kconfig dependencies in PECI subsystem.
>> - Fixed minor bugs and style issues.
>> - configfs support isn't added in this patch set. Will add that using a
>>    seperate patch set.
> 
>> +config PECI
>> +	tristate "PECI support"
>> +	select CRC8
> 
>> +	default n
> 
> As for beginning, this one is redundant.
> If you have more, drop them.

I see. I'll drop the default setting.

>> +#include <linux/bitfield.h>
>> +#include <linux/crc8.h>
>> +#include <linux/delay.h>
>> +#include <linux/mm.h>
>> +#include <linux/module.h>
> 
>> +#include <linux/of_device.h>
> 
> What about ACPI? Can you use fwnode API?

Currently, it's targeting BMC (Baseboard Management Controller) SoCs
that are running on ARM kernel. If any needs of ACPI support comes in
the future, the ACPI support will be added.

Thanks a lot for your review!

-Jae

>> +#include <linux/peci.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/sched/task_stack.h>
>> +#include <linux/slab.h>
> 


More information about the openbmc mailing list