Re: [PATCH v5] Add Silicom Platform Driver

[Guenter Roeck]

On Mon, Aug 28, 2023 at 05:26:22PM -0400, Henry Shi wrote:
> The Silicom platform (silicom-platform) Linux driver for Swisscom
> Business Box (Swisscom BB) as well as Cordoba family products is a 
> software solution designed to facilitate the efficient management
> and control of devices through the integration of various Linux
> frameworks. This platform driver provides seamless support for
> device management via the Linux LED framework, GPIO framework,
> Hardware Monitoring (HWMON), and device attributes. The Silicom
> platform driver's compatibility with these Linux frameworks allows
> applications to access and control Cordoba family devices using
> existing software that is compatible with these frameworks. This
> compatibility simplifies the development process, reduces
> dependencies on proprietary solutions, and promotes
> interoperability with other Linux-based systems and software.
> 
> Signed-off-by: Henry Shi <henryshi2018@xxxxxxxxx>

Again, my feedback is only for hwmon code.

[ ... ]

> +
> +static int silicom_fan_control_read(struct device *dev,
> +									enum hwmon_sensor_types type,
> +									u32 attr, int channel,
> +									long *val)

Excessively long continuation lines.
That seeme to be the case for almost all continuation lines,
except where it is too short. I'd suggest to run the patch
through checkpatch --strict and fix what it reports.

total: 0 errors, 9 warnings, 18 checks, 1077 lines checked

is really a bit much.

[ ... ]

> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(&device->dev, name, NULL,
> +				&silicom_chip_info, NULL);

Did you try to compile this with CONFIG_HWMON=n or with
CONFIG_HWMON=m and SILICOM_PLATFORM=y ?

Guenter