[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <605b729b-3507-4b19-a7a2-229eff2bd424@infradead.org>
Date: Mon, 13 Nov 2023 16:19:08 -0800
From: Randy Dunlap <rdunlap@...radead.org>
To: Henry Shi <henryshi2018@...il.com>, hbshi69@...mail.com,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
hdegoede@...hat.com, markgross@...nel.org, jdelvare@...e.com,
linux@...ck-us.net, linux-kernel@...r.kernel.org,
platform-driver-x86@...r.kernel.org, linux-hwmon@...r.kernel.org
Cc: hb_shi2003@...oo.com, henrys@...icom-usa.com, wenw@...icom-usa.com
Subject: Re: [PATCH v12] platform/x86: Add Silicom Platform Driver
Hi,
Please see comments inline.
On 11/13/23 13:02, Henry Shi wrote:
> platform/x86: Add Silicom Platform Driver
>
> Add Silicom platform (silicom-platform) Linux driver for Swisscom
> Business Box (Swisscom BB) as well as Cordoba family products.
>
> This platform driver provides support for various functions via
> the Linux LED framework, GPIO framework, Hardware Monitoring (HWMON)
> and device attributes.
>
> Signed-off-by: Henry Shi <henryshi2018@...il.com>
> ---
>
[detailed changelog]
> change from patch v11 to v12
> ===========================================
>
> Changes suggested by Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>:
> Several editorial things:
> .Put the subdir headers separately.
> .Add more new line in define section.
> .Use () around all macro arguments to be on the safe side.
> .Add new line and remove comment in function silicom_mec_port_get().
> .Add new line and remove comment in function silicom_mec_port_set().
> .Remove unnecessary comment in function temp_get(), rpm_get().
>
> .../ABI/testing/sysfs-platform-silicom | 262 +++++
> drivers/platform/x86/Kconfig | 14 +
> drivers/platform/x86/Makefile | 3 +
> drivers/platform/x86/silicom-platform.c | 1010 +++++++++++++++++
> 4 files changed, 1289 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-silicom
> create mode 100644 drivers/platform/x86/silicom-platform.c
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-silicom b/Documentation/ABI/testing/sysfs-platform-silicom
> new file mode 100644
> index 000000000000..90d69f0f14c1
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-silicom
> @@ -0,0 +1,262 @@
> +What: /sys/devices/platform/silicom-platform/uc_version
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file allows to read microcontroller firmware
> + version of current platform.
> +
> +What: /sys/devices/platform/silicom-platform/power_cycle
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> + This file allow user to power cycle the platform.
> + default value is 0; when set to 1, it powers down
Default
> + the platform, wait 5 seconds, then power on the
powers
> + device.
> +
> +What: /sys/devices/platform/silicom-platform/efuse_status
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file is read only. It returns the current
> + OTP status:
> +
> + 0 - not programmed.
> + 1 - programmed.
> +
> +What: /sys/devices/platform/silicom-platform/hwmon/hwmon1/temp1_input
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file is read only. It returns the temperature
> + of device.
on what scale? in what units?
> +
> +What: /sys/devices/platform/silicom-platform/hwmon/hwmon1/temp1_label
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file is read only. It returns "Silicom_platform:
> + Thermostat Sensor".
> +
> +What: /sys/devices/platform/silicom-platform/hwmon/hwmon1/fan1_input
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file is read only. It returns current fan
> + speed (RPM).
> +
> +What: /sys/devices/platform/silicom-platform/hwmon/hwmon1/fan1_label
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This file is read only. It returns "Silicom_platform:
> + Fan Speed".
> +
> +What: /sys/class/leds/multicolor:sys/brightness
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + status of system LED brightness:
> +
> + 0 - to turn off the LED
> + 1 - to turn on the LED
> +
> +What: /sys/class/leds/multicolor:sys/multi_index
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read only file. It returns:
> +
> + white amber red
> +
> +What: /sys/class/leds/multicolor:sys/multi_intensity
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + multi-color intensity of system LED: First value for
> + color white; Second value for color amber and third value
color white,
> + for color red:
> +
> + 0 - The color is turned off.
> + 1 - the color is turned on.
> +
> +What: /sys/class/leds/multicolor:wan/brightness
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + status of WAN LED brightness:
> +
> + 0 - to turn off the LED
> + 1 - to turn on the LED
> +
> +What: /sys/class/leds/multicolor:wan/multi_index
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read only file. It returns:
> +
> + white yellow red
> +
> +What: /sys/class/leds/multicolor:wan/multi_intensity
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + multi-color intensity of WAN LED: First value for
> + color white; Second value for color yellow and third value
color white,
> + for color red:
> +
> + 0 - The color is turned off.
> + 1 - the color is turned on.
> +
> +What: /sys/class/leds/multicolor:stat%d/brightness
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + status of device status LED (number %d) brightness:
> +
> + 0 - to turn off the LED
> + 1 - to turn on the LED
> +
> +What: /sys/class/leds/multicolor:stat%d/multi_index
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read only file. It returns:
> +
> + red green blue yellow
> +
> +What: /sys/class/leds/multicolor:stat%d/multi_intensity
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + multi-color intensity of device status LED (number %d):
> + First value for color red; Second value for color green;
> + Third value for color blue and fourth value for color
> + yellow.
> +
> + 0 - The color is turned off.
> + 1 - the color is turned on.
> +
> +What: /sys/class/leds/multicolor:fp_left/brightness
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read/write file. It is used to read/set current
> + status of left LED brightness:
> +
> + 0 - to turn off the LED
> + 1 - to turn on the LED
> +
> +What: /sys/class/leds/multicolor:fp_left/multi_index
> +Date: October 2023
> +KernelVersion: 6.6
> +Contact: Henry Shi <henrys@...icom-usa.com>
> +Description:
> + This is a read only file. It returns:
Drop one space ^^^^^^^^
> +
> + red green blue amber
> +
[]
> diff --git a/drivers/platform/x86/silicom-platform.c b/drivers/platform/x86/silicom-platform.c
> new file mode 100644
> index 000000000000..659fce6584f0
> --- /dev/null
> +++ b/drivers/platform/x86/silicom-platform.c
> @@ -0,0 +1,1010 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// silicom-platform.c - Silicom MEC170x platform driver
> +//
> +// Copyright (C) 2023 Henry Shi <henrys@...icom-usa.com>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/dmi.h>
> +#include <linux/hwmon.h>
> +#include <linux/init.h>
> +#include <linux/ioport.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/led-class-multicolor.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
> +#include <linux/units.h>
> +
> +#include <linux/gpio/driver.h>
> +
> +#define MEC_POWER_CYCLE_ADDR 0x24
> +#define MEC_EFUSE_LSB_ADDR 0x28
> +#define MEC_GPIO_IN_POS 0x08
> +#define MEC_IO_BASE 0x0800
> +#define MEC_IO_LEN 0x8
> +#define IO_REG_BANK 0x0
> +#define DEFAULT_CHAN_LO 0
> +#define DEFAULT_CHAN_HI 0
> +#define DEFAULT_CHAN_LO_T 0xc
> +#define MEC_ADDR ((MEC_IO_BASE) + 0x02)
Extra parens around MEC_IO_BASE are not needed.
[]
> +
> +#define EC_ADDR_MSB ((MEC_IO_BASE) + 0x3)
Ditto.
> +#define MEC_DATA_OFFSET(offset) ((MEC_IO_BASE) + 0x04 + offset)
Ditto.
[]
> +static void silicom_mec_led_mc_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
> + int i;
> +
> + led_mc_calc_color_components(mc_cdev, brightness);
> + for (i = 0; i < mc_cdev->num_colors; i++) {
> + silicom_mec_port_set(mc_cdev->subled_info[i].channel,
> + mc_cdev->subled_info[i].brightness);
Add more indentation on the line above.
> + }
> +}
[]
--
~Randy
Powered by blists - more mailing lists