* Re: [PATCH v3 3/3] DT: leds: Add Qualcomm Light Pulse Generator binding
From: Jacek Anaszewski @ 2017-11-19 21:35 UTC (permalink / raw)
To: Bjorn Andersson, Richard Purdie, Pavel Machek, Rob Herring,
Mark Rutland
Cc: linux-kernel, linux-leds, linux-arm-msm, devicetree, Fenglin Wu
In-Reply-To: <20171115071345.24331-4-bjorn.andersson@linaro.org>
Hi Bjorn,
Thanks for the update.
On 11/15/2017 08:13 AM, Bjorn Andersson wrote:
> This adds the binding document describing the three hardware blocks
> related to the Light Pulse Generator found in a wide range of Qualcomm
> PMICs.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - Squashed all things into one node
> - Removed quirks from the binding, compatible implies number of channels, their
> configuration etc.
> - Binding describes LEDs connected as child nodes
> - Support describing multi-channel LEDs
> - Change style of the binding document, to match other LED bindings
>
> Changes since v1:
> - Dropped custom pattern properties
> - Renamed cell-index to qcom,lpg-channel to clarify its purpose
>
> .../devicetree/bindings/leds/leds-qcom-lpg.txt | 66 ++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
> new file mode 100644
> index 000000000000..9cee6f9f543c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.txt
> @@ -0,0 +1,66 @@
> +Binding for Qualcomm Light Pulse Generator
> +
> +The Qualcomm Light Pulse Generator consists of three different hardware blocks;
> +a ramp generator with lookup table, the light pulse generator and a three
> +channel current sink. These blocks are found in a wide range of Qualcomm PMICs.
> +
> +Required properties:
> +- compatible: one of:
> + "qcom,pm8916-pwm",
> + "qcom,pm8941-lpg",
> + "qcom,pm8994-lpg",
> + "qcom,pmi8994-lpg",
> + "qcom,pmi8998-lpg",
> +
> +Optional properties:
> +- qcom,power-source: power-source used to drive the output, as defined in the
> + datasheet. Should be specified if the TRILED block is
> + present
Range of possible values is missing here.
> +- qcom,dtest: configures the output into an internal test line of the
> + pmic. Specified by a list of u32 pairs, one pair per channel,
> + where each pair denotes the test line to drive and the second
> + configures how the value should be outputed, as defined in the
> + datasheet
> +- #pwm-cells: should be 2, see ../pwm/pwm.txt
> +
> +LED subnodes:
> +A set of subnodes can be used to specify LEDs connected to the LPG. Channels
> +not associated with a LED are available as pwm channels, see ../pwm/pwm.txt.
> +
> +Required properties:
> +- led-sources: list of channels associated with this LED, starting at 1 for the
> + first LPG channel
> +
> +Optional properties:
> +- label: see Documentation/devicetree/bindings/leds/common.txt
> +- default-state: see Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger: see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example:
> +The following example defines a RGB LED attached to the PM8941.
> +
> +&spmi_bus {
> + pm8941@1 {
> + lpg {
> + compatible = "qcom,pm8941-lpg";
> + qcom,power-source = <1>;
> +
> + rgb {
> + led-sources = <7 6 5>;
> + };
> + };
> + };
> +};
> +
> +The following example defines the single PWM channel of the PM8916, which can
> +be muxed by the MPP4 as a current sink.
> +
> +&spmi_bus {
> + pm8916@1 {
> + pm8916_pwm: pwm {
> + compatible = "qcom,pm8916-pwm";
> +
> + #pwm-cells = <2>;
LED has to be represented as a child node -
see Documentation/devicetree/bindings/leds/common.txt
> + };
> + };
> +};
>
Could you please also provide an example of the arrangement on the
board DragonBoard820c, you were describing in the discussions under
the previous version of the patch set. i.e. three green LEDs connected
to TRILED and one to the GPIO sink?
Also any other non-trivial board configurations supported by the
driver would allow to increase our comprehensions of the device
capabilities.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v3 2/3] leds: Add driver for Qualcomm LPG
From: Jacek Anaszewski @ 2017-11-19 21:36 UTC (permalink / raw)
To: Bjorn Andersson, Richard Purdie, Pavel Machek
Cc: linux-kernel, linux-leds, linux-arm-msm, Rob Herring,
Mark Rutland, devicetree, Fenglin Wu
In-Reply-To: <20171115071345.24331-3-bjorn.andersson@linaro.org>
Hi Bjorn,
Thanks for the patch. Please refer to my comments in the code.
On 11/15/2017 08:13 AM, Bjorn Andersson wrote:
> The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> PMICs from Qualcomm. It can operate on fixed parameters or based on a
> lookup-table, altering the duty cycle over time - which provides the
> means for e.g. hardware assisted transitions of LED brightness.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> Changes since v2:
> - Squash all components into one driver
> - Track PWM channels and "logical" LEDs separately
> - Support multiple channels to be bound to a single LED
> - Per-PMIC compatible, to deal with minor differences (e.g. value to enable
> 9bit resolution for PWM)
> - TRILED enablement is done atomically for all channels associated with a LED
> - LUT sequencer start is done atomically for all channels associated with a LED
> - Support PM8916 (PWM only), PM8941, PM8994 and PMI8998 introduced (PMI8994
> still works...)
>
> The multiple channels per LED is currently implemented by assigning the same
> pattern and same brightness to all channels. This allows the RGB LED to show
> various brighness of white and do patterns in shades of white. But it's
> implemented in a way that as we figure out how to expose multi-color LEDs
> through the LED framework this new information could easily be applied to the
> right channel, and we would have the ability to control the channels
> individually.
>
> Changes since v1:
> - Remove custom DT properties for patterns
> - Extract pattern interface into the LED core
>
> drivers/leds/Kconfig | 7 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-qcom-lpg.c | 1232 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1240 insertions(+)
> create mode 100644 drivers/leds/leds-qcom-lpg.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 52ea34e337cd..ccc3aa4b2474 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -651,6 +651,13 @@ config LEDS_POWERNV
> To compile this driver as a module, choose 'm' here: the module
> will be called leds-powernv.
>
> +config LEDS_QCOM_LPG
> + tristate "LED support for Qualcomm LPG"
> + depends on LEDS_CLASS
You were mentioning that this driver is for a MFD child block,
so we should probably depend on the parent MFD driver?
> + help
> + This option enables support for the Light Pulse Generator found in a
> + wide variety of Qualcomm PMICs.
> +
> config LEDS_SYSCON
> bool "LED support for LEDs on system controllers"
> depends on LEDS_CLASS=y
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 35980450db9b..2d5149ca429d 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -63,6 +63,7 @@ obj-$(CONFIG_LEDS_MAX77693) += leds-max77693.o
> obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> obj-$(CONFIG_LEDS_LM355x) += leds-lm355x.o
> obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o
> +obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o
> obj-$(CONFIG_LEDS_SYSCON) += leds-syscon.o
> obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> obj-$(CONFIG_LEDS_KTD2692) += leds-ktd2692.o
> diff --git a/drivers/leds/leds-qcom-lpg.c b/drivers/leds/leds-qcom-lpg.c
> new file mode 100644
> index 000000000000..481e940d7e04
> --- /dev/null
> +++ b/drivers/leds/leds-qcom-lpg.c
> @@ -0,0 +1,1232 @@
> +/*
> + * Copyright (c) 2017 Linaro Ltd
> + * Copyright (c) 2010-2012, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define LPG_PATTERN_CONFIG_REG 0x40
> +#define LPG_SIZE_CLK_REG 0x41
> +#define LPG_PREDIV_CLK_REG 0x42
> +#define PWM_TYPE_CONFIG_REG 0x43
> +#define PWM_VALUE_REG 0x44
> +#define PWM_ENABLE_CONTROL_REG 0x46
> +#define PWM_SYNC_REG 0x47
> +#define LPG_RAMP_DURATION_REG 0x50
> +#define LPG_HI_PAUSE_REG 0x52
> +#define LPG_LO_PAUSE_REG 0x54
> +#define LPG_HI_IDX_REG 0x56
> +#define LPG_LO_IDX_REG 0x57
> +#define PWM_SEC_ACCESS_REG 0xd0
> +#define PWM_DTEST_REG(x) (0xe2 + (x) - 1)
> +
> +#define TRI_LED_SRC_SEL 0x45
> +#define TRI_LED_EN_CTL 0x46
> +#define TRI_LED_ATC_CTL 0x47
> +
> +#define LPG_LUT_REG(x) (0x40 + (x) * 2)
> +#define RAMP_CONTROL_REG 0xc8
Please add QCOM_ namespacing prefix to the macros.
At least PWM prefix is reserved for pwm subsystem.
> +
> +struct lpg_channel;
> +struct lpg_data;
> +
> +/**
> + * struct lpg - LPG device context
> + * @dev: struct device for LPG device
> + * @map: regmap for register access
> + * @pwm: PWM-chip object, if operating in PWM mode
> + * @pwm_9bit_mask: bitmask for enabling 9bit pwm
> + * @lut_base: base address of the LUT block (optional)
> + * @lut_size: number of entries in the LUT block
> + * @lut_bitmap: allocation bitmap for LUT entries
> + * @triled_base: base address of the TRILED block (optional)
> + * @triled_src: power-source for the TRILED
> + * @channels: list of PWM channels
> + * @num_channels: number of @channels
> + */
> +struct lpg {
> + struct device *dev;
> + struct regmap *map;
> +
> + struct pwm_chip pwm;
> +
> + const struct lpg_data *data;
> +
> + u32 lut_base;
> + u32 lut_size;
> + unsigned long *lut_bitmap;
> +
> + u32 triled_base;
> + u32 triled_src;
> +
> + struct lpg_channel *channels;
> + unsigned int num_channels;
> +};
> +
> +/**
> + * struct lpg_channel - per channel data
> + * @lpg: reference to parent lpg
> + * @base: base address of the PWM channel
> + * @triled_mask: mask in TRILED to enable this channel
> + * @lut_mask: mask in LUT to start pattern generator for this channel
> + * @in_use: channel is exposed to LED framework
> + * @dtest_line: DTEST line for output, or 0 if disabled
> + * @dtest_value: DTEST line configuration
> + * @pwm_value: duty (in microseconds) of the generated pulses, overriden by LUT
> + * @enabled: output enabled?
> + * @period_us: period (in microseconds) of the generated pulses
> + * @pwm_size: resolution of the @pwm_value, 6 or 9 bits
> + * @clk: base frequency of the clock generator
> + * @pre_div: divider of @clk
> + * @pre_div_exp: exponential divider of @clk
> + * @ramp_enabled: duty cycle is driven by iterating over lookup table
> + * @ramp_ping_pong: reverse through pattern, rather than wrapping to start
> + * @ramp_oneshot: perform only a single pass over the pattern
> + * @ramp_reverse: iterate over pattern backwards
> + * @ramp_duration_ms: length (in milliseconds) of one pattern run
> + * @ramp_lo_pause_ms: pause (in milliseconds) before iterating over pattern
> + * @ramp_hi_pause_ms: pause (in milliseconds) after iterating over pattern
> + * @pattern_lo_idx: start index of associated pattern
> + * @pattern_hi_idx: last index of associated pattern
> + */
> +struct lpg_channel {
> + struct lpg *lpg;
> +
> + u32 base;
> + unsigned int triled_mask;
> + unsigned int lut_mask;
> +
> + bool in_use;
> +
> + u32 dtest_line;
> + u32 dtest_value;
> +
> + u16 pwm_value;
> + bool enabled;
> +
> + unsigned int period_us;
> + unsigned int pwm_size;
> + unsigned int clk;
> + unsigned int pre_div;
> + unsigned int pre_div_exp;
> +
> + bool ramp_enabled;
> + bool ramp_ping_pong;
> + bool ramp_oneshot;
> + bool ramp_reverse;
> + unsigned long ramp_duration_ms;
> + unsigned long ramp_lo_pause_ms;
> + unsigned long ramp_hi_pause_ms;
> +
> + unsigned int pattern_lo_idx;
> + unsigned int pattern_hi_idx;
> +};
> +
> +/**
> + * struct lpg_led - logical LED object
> + * @lpg: lpg context reference
> + * @cdev: LED class device
> + * @num_channels: number of @channels
> + * @channels: list of channels associated with the LED
> + */
> +struct lpg_led {
> + struct lpg *lpg;
> +
> + struct led_classdev cdev;
> +
> + unsigned int num_channels;
> + struct lpg_channel *channels[];
> +};
> +
> +/**
> + * struct lpg_channel_data - per channel initialization data
> + * @base: base address for PWM channel registers
> + * @triled_mask: bitmask for controlling this channel in TRILED
> + */
> +struct lpg_channel_data {
> + unsigned int base;
> + u8 triled_mask;
> +};
> +
> +/**
> + * struct lpg_data - initialization data
> + * @lut_base: base address of LUT block
> + * @lut_size: number of entries in LUT
> + * @triled_base: base address of TRILED
> + * @pwm_9bit_mask: bitmask for switching from 6bit to 9bit pwm
> + * @num_channels: number of channels in LPG
> + * @channels: list of channel initialization data
> + */
> +struct lpg_data {
> + unsigned int lut_base;
> + unsigned int lut_size;
> + unsigned int triled_base;
> + unsigned int pwm_9bit_mask;
> + int num_channels;
> + struct lpg_channel_data *channels;
> +};
> +
> +static int triled_set(struct lpg *lpg, unsigned int mask, bool enable)
> +{
> + /* Skip if we don't have a triled block */
> + if (!lpg->triled_base)
> + return 0;
> +
> + return regmap_update_bits(lpg->map, lpg->triled_base + TRI_LED_EN_CTL,
> + mask, enable ? mask : 0);
> +}
> +
> +static int lpg_lut_store(struct lpg *lpg, const u16 *values, size_t len,
> + unsigned int *lo_idx, unsigned int *hi_idx)
> +{
> + unsigned int idx;
> + u8 val[2];
> + int i;
> +
> + /* Hardware does not behave when LO_IDX == HI_IDX */
> + if (len == 1)
> + return -EINVAL;
> +
> + idx = bitmap_find_next_zero_area(lpg->lut_bitmap, lpg->lut_size,
> + 0, len, 0);
> + if (idx >= lpg->lut_size)
> + return -ENOMEM;
> +
> + for (i = 0; i < len; i++) {
> + val[0] = values[i] & 0xff;
> + val[1] = values[i] >> 8;
> +
> + regmap_bulk_write(lpg->map,
> + lpg->lut_base + LPG_LUT_REG(idx + i), val, 2);
> + }
> +
> + bitmap_set(lpg->lut_bitmap, idx, len);
> +
> + *lo_idx = idx;
> + *hi_idx = idx + len - 1;
> +
> + return 0;
> +}
> +
> +static u16 *lpg_lut_read(struct lpg *lpg, unsigned int lo_idx,
> + unsigned int hi_idx, size_t *len)
> +{
> + u16 *values;
> + u8 val[2];
> + int ret;
> + int i;
> +
> + *len = hi_idx - lo_idx + 1;
> +
> + values = kcalloc(*len, sizeof(u16), GFP_KERNEL);
> + if (!values)
> + return ERR_PTR(-ENOMEM);
> +
> + for (i = 0; i < *len; i++) {
> + ret = regmap_bulk_read(lpg->map,
> + lpg->lut_base + LPG_LUT_REG(lo_idx + i),
> + &val, 2);
> + if (ret < 0) {
> + kfree(values);
> + return ERR_PTR(ret);
> + }
> +
> + values[i] = val[0] | val[1] << 8;
> + }
> +
> + return values;
> +}
> +
> +static void lpg_lut_free(struct lpg *lpg, unsigned int lo_idx, unsigned int hi_idx)
> +{
> + int len;
> +
> + if (lo_idx == hi_idx)
> + return;
> +
> + len = hi_idx - lo_idx + 1;
> + bitmap_clear(lpg->lut_bitmap, lo_idx, len);
> +}
> +
> +static int lpg_lut_sync(struct lpg *lpg, unsigned int mask)
> +{
> + return regmap_update_bits(lpg->map, lpg->lut_base + RAMP_CONTROL_REG,
> + mask, 0xff);
> +}
> +
> +#define NUM_PWM_PREDIV 4
> +#define NUM_PWM_CLK 3
> +#define NUM_EXP 7
> +
> +static const unsigned int lpg_clk_table[NUM_PWM_PREDIV][NUM_PWM_CLK] = {
> + {
> + 1 * (NSEC_PER_SEC / 1024),
> + 1 * (NSEC_PER_SEC / 32768),
> + 1 * (NSEC_PER_SEC / 19200000),
> + },
> + {
> + 3 * (NSEC_PER_SEC / 1024),
> + 3 * (NSEC_PER_SEC / 32768),
> + 3 * (NSEC_PER_SEC / 19200000),
> + },
> + {
> + 5 * (NSEC_PER_SEC / 1024),
> + 5 * (NSEC_PER_SEC / 32768),
> + 5 * (NSEC_PER_SEC / 19200000),
> + },
> + {
> + 6 * (NSEC_PER_SEC / 1024),
> + 6 * (NSEC_PER_SEC / 32768),
> + 6 * (NSEC_PER_SEC / 19200000),
> + },
> +};
> +
> +/*
> + * PWM Frequency = Clock Frequency / (N * T)
> + * or
> + * PWM Period = Clock Period * (N * T)
> + * where
> + * N = 2^9 or 2^6 for 9-bit or 6-bit PWM size
> + * T = Pre-divide * 2^m, where m = 0..7 (exponent)
> + *
> + * This is the formula to figure out m for the best pre-divide and clock:
> + * (PWM Period / N) = (Pre-divide * Clock Period) * 2^m
> + */
> +static void lpg_calc_freq(struct lpg_channel *chan, unsigned int period_us)
> +{
> + int n, m, clk, div;
> + int best_m, best_div, best_clk;
> + unsigned int last_err, cur_err, min_err;
> + unsigned int tmp_p, period_n;
> +
> + if (period_us == chan->period_us)
> + return;
> +
> + /* PWM Period / N */
> + if (period_us < ((unsigned int)(-1) / NSEC_PER_USEC)) {
> + period_n = (period_us * NSEC_PER_USEC) >> 6;
> + n = 6;
> + } else {
> + period_n = (period_us >> 9) * NSEC_PER_USEC;
> + n = 9;
> + }
Please provide macros for 6 and 9 magic numbers.
> +
> + min_err = last_err = (unsigned int)(-1);
> + best_m = 0;
> + best_clk = 0;
> + best_div = 0;
> + for (clk = 0; clk < NUM_PWM_CLK; clk++) {
> + for (div = 0; div < NUM_PWM_PREDIV; div++) {
> + /* period_n = (PWM Period / N) */
> + /* tmp_p = (Pre-divide * Clock Period) * 2^m */
> + tmp_p = lpg_clk_table[div][clk];
> + for (m = 0; m <= NUM_EXP; m++) {
> + if (period_n > tmp_p)
> + cur_err = period_n - tmp_p;
> + else
> + cur_err = tmp_p - period_n;
> +
> + if (cur_err < min_err) {
> + min_err = cur_err;
> + best_m = m;
> + best_clk = clk;
> + best_div = div;
> + }
> +
> + if (m && cur_err > last_err)
> + /* Break for bigger cur_err */
> + break;
> +
> + last_err = cur_err;
> + tmp_p <<= 1;
> + }
> + }
> + }
> +
> + /* Use higher resolution */
> + if (best_m >= 3 && n == 6) {
> + n += 3;
> + best_m -= 3;
> + }
> +
> + chan->clk = best_clk;
> + chan->pre_div = best_div;
> + chan->pre_div_exp = best_m;
> + chan->pwm_size = n;
> +
> + chan->period_us = period_us;
> +}
> +
> +static void lpg_calc_duty(struct lpg_channel *chan, unsigned int duty_us)
> +{
> + unsigned long max = (1 << chan->pwm_size) - 1;
> + unsigned long val;
> +
> + /* Figure out pwm_value with overflow handling */
> + if (duty_us < 1 << (sizeof(val) * 8 - chan->pwm_size))
> + val = (duty_us << chan->pwm_size) / chan->period_us;
> + else
> + val = duty_us / (chan->period_us >> chan->pwm_size);
> +
> + if (val > max)
> + val = max;
> +
> + chan->pwm_value = val;
> +}
> +
> +static void lpg_apply_freq(struct lpg_channel *chan)
> +{
> + unsigned long val;
> + struct lpg *lpg = chan->lpg;
> +
> + if (!chan->enabled)
> + return;
> +
> + /* Clock register values are off-by-one from lpg_clk_table */
> + val = chan->clk + 1;
> +
> + if (chan->pwm_size == 9)
> + val |= lpg->data->pwm_9bit_mask;
> +
> + regmap_write(lpg->map, chan->base + LPG_SIZE_CLK_REG, val);
> +
> + val = chan->pre_div << 5 | chan->pre_div_exp;
> + regmap_write(lpg->map, chan->base + LPG_PREDIV_CLK_REG, val);
Please provide macros for 5 and 9.
> +}
> +
> +#define LPG_ENABLE_GLITCH_REMOVAL BIT(5)
> +
> +static void lpg_enable_glitch(struct lpg_channel *chan)
> +{
> + struct lpg *lpg = chan->lpg;
> +
> + regmap_update_bits(lpg->map, chan->base + PWM_TYPE_CONFIG_REG,
> + LPG_ENABLE_GLITCH_REMOVAL, 0);
> +}
> +
> +static void lpg_disable_glitch(struct lpg_channel *chan)
> +{
> + struct lpg *lpg = chan->lpg;
> +
> + regmap_update_bits(lpg->map, chan->base + PWM_TYPE_CONFIG_REG,
> + LPG_ENABLE_GLITCH_REMOVAL,
> + LPG_ENABLE_GLITCH_REMOVAL);
> +}
> +
> +static void lpg_apply_pwm_value(struct lpg_channel *chan)
> +{
> + u8 val[] = { chan->pwm_value & 0xff, chan->pwm_value >> 8 };
> + struct lpg *lpg = chan->lpg;
> +
> + if (!chan->enabled)
> + return;
> +
> + regmap_bulk_write(lpg->map, chan->base + PWM_VALUE_REG, val, 2);
> +}
> +
> +#define LPG_PATTERN_CONFIG_LO_TO_HI BIT(4)
> +#define LPG_PATTERN_CONFIG_REPEAT BIT(3)
> +#define LPG_PATTERN_CONFIG_TOGGLE BIT(2)
> +#define LPG_PATTERN_CONFIG_PAUSE_HI BIT(1)
> +#define LPG_PATTERN_CONFIG_PAUSE_LO BIT(0)
> +
> +static void lpg_apply_lut_control(struct lpg_channel *chan)
> +{
> + struct lpg *lpg = chan->lpg;
> + unsigned int hi_pause;
> + unsigned int lo_pause;
> + unsigned int step;
> + unsigned int conf = 0;
> + unsigned int lo_idx = chan->pattern_lo_idx;
> + unsigned int hi_idx = chan->pattern_hi_idx;
> + int pattern_len;
> +
> + if (!chan->ramp_enabled || chan->pattern_lo_idx == chan->pattern_hi_idx)
> + return;
> +
> + pattern_len = hi_idx - lo_idx + 1;
> +
> + step = DIV_ROUND_UP(chan->ramp_duration_ms, pattern_len);
> + hi_pause = DIV_ROUND_UP(chan->ramp_hi_pause_ms, step);
> + lo_pause = DIV_ROUND_UP(chan->ramp_lo_pause_ms, step);
> +
> + if (!chan->ramp_reverse)
> + conf |= LPG_PATTERN_CONFIG_LO_TO_HI;
> + if (!chan->ramp_oneshot)
> + conf |= LPG_PATTERN_CONFIG_REPEAT;
> + if (chan->ramp_ping_pong)
> + conf |= LPG_PATTERN_CONFIG_TOGGLE;
> + if (chan->ramp_hi_pause_ms)
> + conf |= LPG_PATTERN_CONFIG_PAUSE_HI;
> + if (chan->ramp_lo_pause_ms)
> + conf |= LPG_PATTERN_CONFIG_PAUSE_LO;
> +
> + regmap_write(lpg->map, chan->base + LPG_PATTERN_CONFIG_REG, conf);
> + regmap_write(lpg->map, chan->base + LPG_HI_IDX_REG, hi_idx);
> + regmap_write(lpg->map, chan->base + LPG_LO_IDX_REG, lo_idx);
> +
> + regmap_write(lpg->map, chan->base + LPG_RAMP_DURATION_REG, step);
> + regmap_write(lpg->map, chan->base + LPG_HI_PAUSE_REG, hi_pause);
> + regmap_write(lpg->map, chan->base + LPG_LO_PAUSE_REG, lo_pause);
> +}
> +
> +#define LPG_ENABLE_CONTROL_OUTPUT BIT(7)
> +#define LPG_ENABLE_CONTROL_BUFFER_TRISTATE BIT(5)
> +#define LPG_ENABLE_CONTROL_SRC_PWM BIT(2)
> +#define LPG_ENABLE_CONTROL_RAMP_GEN BIT(1)
> +
> +static void lpg_apply_control(struct lpg_channel *chan)
> +{
> + unsigned int ctrl;
> + struct lpg *lpg = chan->lpg;
> +
> + ctrl = LPG_ENABLE_CONTROL_BUFFER_TRISTATE;
> +
> + if (chan->enabled)
> + ctrl |= LPG_ENABLE_CONTROL_OUTPUT;
> +
> + if (chan->pattern_lo_idx != chan->pattern_hi_idx)
> + ctrl |= LPG_ENABLE_CONTROL_RAMP_GEN;
> + else
> + ctrl |= LPG_ENABLE_CONTROL_SRC_PWM;
> +
> + regmap_write(lpg->map, chan->base + PWM_ENABLE_CONTROL_REG, ctrl);
> +
> + /*
> + * Due to LPG hardware bug, in the PWM mode, having enabled PWM,
> + * We have to write PWM values one more time.
> + */
> + if (chan->enabled)
> + lpg_apply_pwm_value(chan);
> +}
> +
> +#define LPG_SYNC_PWM BIT(0)
> +
> +static void lpg_apply_sync(struct lpg_channel *chan)
> +{
> + struct lpg *lpg = chan->lpg;
> +
> + regmap_write(lpg->map, chan->base + PWM_SYNC_REG, LPG_SYNC_PWM);
> +}
> +
> +static void lpg_apply_dtest(struct lpg_channel *chan)
> +{
> + struct lpg *lpg = chan->lpg;
> +
> + if (!chan->dtest_line)
> + return;
> +
> + regmap_write(lpg->map, chan->base + PWM_SEC_ACCESS_REG, 0xa5);
> + regmap_write(lpg->map, chan->base + PWM_DTEST_REG(chan->dtest_line),
> + chan->dtest_value);
> +}
> +
> +static void lpg_apply(struct lpg_channel *chan)
> +{
> + lpg_disable_glitch(chan);
> + lpg_apply_freq(chan);
> + lpg_apply_pwm_value(chan);
> + lpg_apply_control(chan);
> + lpg_apply_sync(chan);
> + lpg_apply_lut_control(chan);
> + lpg_enable_glitch(chan);
> +}
> +
> +static void lpg_brightness_set(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
> + struct lpg_channel *chan;
> + struct lpg *lpg = led->lpg;
> + unsigned int duty_us;
> + unsigned int triled_mask = 0;
> + unsigned int lut_mask = 0;
> + int i;
> +
> + for (i = 0; i < led->num_channels; i++) {
> + chan = led->channels[i];
> +
> + if (value == LED_OFF) {
> + chan->enabled = false;
> + chan->ramp_enabled = false;
> + } else if (chan->pattern_lo_idx != chan->pattern_hi_idx) {
> + lpg_calc_freq(chan, NSEC_PER_USEC);
> +
> + chan->enabled = true;
> + chan->ramp_enabled = true;
> +
> + lut_mask |= chan->lut_mask;
> + triled_mask |= chan->triled_mask;
> + } else {
> + lpg_calc_freq(chan, NSEC_PER_USEC);
> +
> + duty_us = value * chan->period_us / cdev->max_brightness;
> + lpg_calc_duty(chan, duty_us);
> + chan->enabled = true;
> + chan->ramp_enabled = false;
> +
> + triled_mask |= chan->triled_mask;
> + }
> +
> + lpg_apply(chan);
> + }
> +
> + /* Toggle triled lines */
> + if (triled_mask)
> + triled_set(lpg, triled_mask, chan->enabled);
> +
> + /* Trigger start of ramp generator(s) */
> + if (lut_mask)
> + lpg_lut_sync(lpg, lut_mask);
We need some synchronization while changing device state in
few steps, to prevent troubles when we are preempted by other
process in the middle. spin_lock() in this case since it seems
that we are not going to sleep while accessing device registers.
> +}
> +
> +static enum led_brightness lpg_brightness_get(struct led_classdev *cdev)
> +{
> + struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
> + struct lpg_channel *chan = led->channels[0];
> + unsigned long max = (1 << chan->pwm_size) - 1;
> +
> + if (!chan->enabled)
> + return LED_OFF;
> + else if (chan->pattern_lo_idx != chan->pattern_hi_idx)
> + return LED_FULL;
> + else
> + return chan->pwm_value * cdev->max_brightness / max;
> +}
> +
> +static int lpg_blink_set(struct led_classdev *cdev,
> + unsigned long *delay_on, unsigned long *delay_off)
> +{
> + struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
> + struct lpg_channel *chan = led->channels[0];
> + unsigned int period_us;
> + unsigned int duty_us;
> +
> + if (!*delay_on && !*delay_off) {
> + *delay_on = 500;
> + *delay_off = 500;
> + }
> +
> + duty_us = *delay_on * USEC_PER_MSEC;
> + period_us = (*delay_on + *delay_off) * USEC_PER_MSEC;
> +
> + lpg_calc_freq(chan, period_us);
> + lpg_calc_duty(chan, duty_us);
> +
> + chan->enabled = true;
> + chan->ramp_enabled = false;
> +
> + lpg_apply(chan);
> +
> + return 0;
> +}
> +
> +#define interpolate(x1, y1, x2, y2, x) \
> + ((y1) + ((y2) - (y1)) * ((x) - (x1)) / ((x2) - (x1)))
> +
> +static int lpg_pattern_set(struct led_classdev *led_cdev,
> + struct led_pattern *led_pattern, int len,
> + bool repeat)
> +{
> + struct lpg_led *led = container_of(led_cdev, struct lpg_led, cdev);
> + struct lpg_channel *chan = led->channels[0];
> + struct lpg *lpg = led->lpg;
> + unsigned int duration = 0;
> + unsigned int min_delta = (unsigned int)-1;
> + unsigned int hi_pause;
> + unsigned int lo_pause = 0;
> + unsigned int lo_idx;
> + unsigned int hi_idx;
> + unsigned int max = (1 << chan->pwm_size) - 1;
> + bool ping_pong = true;
> + int brightness_a;
> + int brightness_b;
> + u16 *pattern;
> + int src_idx;
> + int dst_idx;
> + int step_t;
> + int time_a;
> + int time_b;
> + int value;
> + int steps;
> + int ret = 0;
> + int i;
> +
> + /*
> + * The led_pattern specifies brightness values, potentially distributed
> + * unevenly over the duration of the pattern. The LPG only support
> + * evenly distributed values, so we interpolate new values from the
> + * led_pattern.
> + */
> +
> + /* Sum the duration over the inner delta_ts and the tail is hi_pause */
> + for (src_idx = 0; src_idx < len - 1; src_idx++)
> + duration += led_pattern[src_idx].delta_t;
> + hi_pause = led_pattern[src_idx].delta_t;
> +
> + for (src_idx = 0; src_idx < len; src_idx++) {
> + min_delta = min_t(unsigned int, min_delta,
> + led_pattern[src_idx].delta_t);
> + }
> +
> + steps = duration / min_delta + 1;
> + pattern = kcalloc(steps, sizeof(*pattern), GFP_KERNEL);
> + if (!pattern)
> + return -ENOMEM;
> +
> + time_a = 0;
> + for (src_idx = 0, dst_idx = 0; dst_idx < steps; dst_idx++) {
> + /* The timestamp of this evenly distributed data point */
> + step_t = dst_idx * min_delta;
> +
> + /*
> + * Find time_a - time_b interval from source pattern that spans
> + * step_t
> + */
> + while (time_a + led_pattern[src_idx].delta_t < step_t) {
> + if (src_idx >= len - 1)
> + break;
> + time_a += led_pattern[++src_idx].delta_t;
> + }
> +
> + if (src_idx < len - 1) {
> + time_b = time_a + led_pattern[src_idx].delta_t;
> +
> + brightness_a = led_pattern[src_idx].brightness;
> + brightness_b = led_pattern[src_idx + 1].brightness;
> +
> + /* Interpolate over the source pattern segment */
> + value = interpolate(time_a, brightness_a, time_b,
> + brightness_b, step_t);
> + } else {
> + value = led_pattern[src_idx].brightness;
> + }
> +
> + /* Scale calculated value to the hardware brightness value */
> + pattern[dst_idx] = value * max / led_cdev->max_brightness;
> + }
> +
> + /* Detect palindromes and use "ping pong" to reduce LUT usage */
> + for (dst_idx = 0; dst_idx < steps / 2; dst_idx++) {
> + if (pattern[dst_idx] != pattern[len - dst_idx - 1]) {
> + ping_pong = false;
> + break;
> + }
> + }
> + if (ping_pong) {
> + steps = (steps + 1) / 2;
> +
> + /*
> + * When ping_pong is set the hi_pause will happen in the middle
> + * of the pattern, so we need to use lo_pause to delay between
> + * the loops.
> + */
> + if (repeat)
> + lo_pause = hi_pause;
> +
> + hi_pause = 0;
> + }
> +
> + ret = lpg_lut_store(lpg, pattern, steps, &lo_idx, &hi_idx);
> + if (ret < 0)
> + goto out;
> +
> + chan = led->channels[0];
> +
> + lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
> +
> + /* Update settings on each associated channel */
> + for (i = 0; i < led->num_channels; i++) {
> + chan = led->channels[i];
> +
> + chan->ramp_duration_ms = duration;
> + chan->ramp_ping_pong = ping_pong;
> + chan->ramp_oneshot = !repeat;
> +
> + chan->pattern_lo_idx = lo_idx;
> + chan->pattern_hi_idx = hi_idx;
> + }
> +
> +out:
> + kfree(pattern);
> +
> + return ret;
> +}
> +
> +static int lpg_pattern_clear(struct led_classdev *cdev)
> +{
> + struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
> + struct lpg_channel *chan;
> + struct lpg *lpg = led->lpg;
> + int i;
> +
> + chan = led->channels[0];
> +
> + lpg_lut_free(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx);
> +
> + for (i = 0; i < led->num_channels; i++) {
> + chan = led->channels[i];
> + chan->pattern_lo_idx = 0;
> + chan->pattern_hi_idx = 0;
> + }
> +
> + return 0;
> +}
> +
> +static struct led_pattern *lpg_pattern_get(struct led_classdev *cdev,
> + size_t *len, bool *repeat)
> +{
> + struct led_pattern *led_pattern;
> + struct lpg_led *led = container_of(cdev, struct lpg_led, cdev);
> + struct lpg *lpg = led->lpg;
> + struct lpg_channel *chan = led->channels[0];
> + unsigned int delta_t;
> + unsigned int max = (1 << chan->pwm_size) - 1;
> + size_t all_steps;
> + size_t steps;
> + u16 *pattern;
> + size_t i;
> + u16 val;
> +
> + pattern = lpg_lut_read(lpg, chan->pattern_lo_idx, chan->pattern_hi_idx,
> + &steps);
> + if (IS_ERR_OR_NULL(pattern))
> + return ERR_CAST(pattern);
> +
> + all_steps = chan->ramp_ping_pong ? steps * 2 - 1 : steps;
> +
> + delta_t = (chan->ramp_duration_ms + chan->ramp_hi_pause_ms) / all_steps;
> +
> + led_pattern = kcalloc(all_steps, sizeof(*pattern), GFP_KERNEL);
> + if (!led_pattern) {
> + led_pattern = ERR_PTR(-ENOMEM);
> + goto out;
> + }
> +
> + for (i = 0; i < all_steps; i++) {
> + if (i < steps)
> + val = pattern[i];
> + else
> + val = pattern[steps - i];
> +
> + led_pattern[i].delta_t = delta_t;
> + led_pattern[i].brightness = val * cdev->max_brightness / max;
> + }
> +
> + *len = all_steps;
> + *repeat = !chan->ramp_oneshot;
> +
> +out:
> + kfree(pattern);
> + return led_pattern;
> +}
> +
> +static int lpg_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct lpg *lpg = container_of(chip, struct lpg, pwm);
> + struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
> +
> + return chan->in_use ? -EBUSY : 0;
> +}
> +
> +static int lpg_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + struct pwm_state *state)
> +{
> + struct lpg *lpg = container_of(chip, struct lpg, pwm);
> + struct lpg_channel *chan = &lpg->channels[pwm->hwpwm];
> +
> + lpg_calc_freq(chan, state->period / NSEC_PER_USEC);
> + lpg_calc_duty(chan, state->duty_cycle / NSEC_PER_USEC);
> + chan->enabled = state->enabled;
> +
> + lpg_apply(chan);
> +
> + triled_set(lpg, chan->triled_mask, chan->enabled);
> +
> + state->polarity = PWM_POLARITY_NORMAL;
> + state->period = chan->period_us * NSEC_PER_USEC;
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops lpg_pwm_ops = {
> + .request = lpg_pwm_request,
> + .apply = lpg_pwm_apply,
> + .owner = THIS_MODULE,
> +};
> +
> +static int lpg_add_pwm(struct lpg *lpg)
> +{
> + int ret;
> +
> + lpg->pwm.base = -1;
> + lpg->pwm.dev = lpg->dev;
> + lpg->pwm.npwm = lpg->num_channels;
> + lpg->pwm.ops = &lpg_pwm_ops;
> +
> + ret = pwmchip_add(&lpg->pwm);
> + if (ret)
> + dev_err(lpg->dev, "failed to add PWM chip: ret %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int lpg_add_led(struct lpg *lpg, struct device_node *np)
> +{
> + struct lpg_led *led;
> + const char *state;
> + int sources;
> + int size;
> + u32 chan;
> + int ret;
> + int i;
> +
> + sources = of_property_count_u32_elems(np, "led-sources");
> + if (sources <= 0) {
> + dev_err(lpg->dev, "invalid led-sources of %s\n",
> + np->name);
> + return -EINVAL;
> + }
> +
> + size = sizeof(*led) + sources * sizeof(struct lpg_channel*);
To fix checkpatch.pl complaint:
s/lpg_channel*/lpg_channel */
> + led = devm_kzalloc(lpg->dev, size, GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + led->lpg = lpg;
> + led->num_channels = sources;
> +
> + for (i = 0; i < sources; i++) {
> + ret = of_property_read_u32_index(np, "led-sources",
> + i, &chan);
> + if (ret || !chan || chan > lpg->num_channels) {
> + dev_err(lpg->dev,
> + "invalid led-sources of %s\n",
> + np->name);
> + return -EINVAL;
> + }
> +
> + led->channels[i] = &lpg->channels[chan - 1];
> +
> + led->channels[i]->in_use = true;
> + }
> +
> + /* Use label else node name */
> + led->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
Documentation/leds/leds-class.txt states that LED class device name
pattern is devicename:colour:function.
This is not explicitly stated in the common LED DT bindings, but label
should be prepended with devicename by the driver. Not all LED class
drivers adhere to this rule and we have some mess in this area
currently, but we will fix it soon I hope.
> + led->cdev.default_trigger = of_get_property(np, "linux,default-trigger", NULL);
> + led->cdev.brightness_set = lpg_brightness_set;
> + led->cdev.brightness_get = lpg_brightness_get;
> + led->cdev.blink_set = lpg_blink_set;
> + led->cdev.max_brightness = 255;
You can skip this line, since it will be set to LED_FULL
in case passed 0 to led_classdev_init().
> +
> + /* Register pattern accessors only if we have a LUT block */
> + if (lpg->lut_base) {
> + led->cdev.pattern_set = lpg_pattern_set;
> + led->cdev.pattern_clear = lpg_pattern_clear;
> + led->cdev.pattern_get = lpg_pattern_get;
> + }
> +
> + if (!of_property_read_string(np, "default-state", &state) &&
> + !strcmp(state, "on"))
> + led->cdev.brightness = LED_FULL;
> + else
> + led->cdev.brightness = LED_OFF;
> +
> + lpg_brightness_set(&led->cdev, led->cdev.brightness);
> +
> + ret = devm_led_classdev_register(lpg->dev, &led->cdev);
> + if (ret)
> + dev_err(lpg->dev, "unable to register %s\n", led->cdev.name);
> +
> + return ret;
> +}
> +
> +static int lpg_init_channels(struct lpg *lpg)
> +{
> + const struct lpg_data *data = lpg->data;
> + int i;
> +
> + lpg->num_channels = data->num_channels;
> + lpg->channels = devm_kcalloc(lpg->dev, data->num_channels,
> + sizeof(struct lpg_channel), GFP_KERNEL);
> + if (!lpg->channels)
> + return -ENOMEM;
> +
> + for (i = 0; i < data->num_channels; i++) {
> + lpg->channels[i].lpg = lpg;
> + lpg->channels[i].base = data->channels[i].base;
> + lpg->channels[i].triled_mask = data->channels[i].triled_mask;
> + lpg->channels[i].lut_mask = BIT(i);
> + }
> +
> + return 0;
> +}
> +
> +static int lpg_init_triled(struct lpg *lpg)
> +{
> + struct device_node *np = lpg->dev->of_node;
> + int ret;
> +
> + /* Skip initialization if we don't have a triled block */
> + if (!lpg->data->triled_base)
> + return 0;
> +
> + lpg->triled_base = lpg->data->triled_base;
> +
> + ret = of_property_read_u32(np, "qcom,power-source", &lpg->triled_src);
> + if (ret || lpg->triled_src == 2 || lpg->triled_src > 3) {
> + dev_err(lpg->dev, "invalid power source\n");
> + return -EINVAL;
> + }
> +
> + /* Disable automatic trickle charge LED */
> + regmap_write(lpg->map, lpg->triled_base + TRI_LED_ATC_CTL, 0);
> +
> + /* Configure power source */
> + regmap_write(lpg->map, lpg->triled_base + TRI_LED_SRC_SEL,
> + lpg->triled_src);
> +
> + /* Default all outputs to off */
> + regmap_write(lpg->map, lpg->triled_base + TRI_LED_EN_CTL, 0);
> +
> + return 0;
> +}
> +
> +static int lpg_init_lut(struct lpg *lpg)
> +{
> + const struct lpg_data *data = lpg->data;
> + size_t bitmap_size;
> +
> + if (!data->lut_base)
> + return 0;
> +
> + lpg->lut_base = data->lut_base;
> + lpg->lut_size = data->lut_size;
> +
> + bitmap_size = BITS_TO_LONGS(lpg->lut_size) / sizeof(unsigned long);
> + lpg->lut_bitmap = devm_kzalloc(lpg->dev, bitmap_size, GFP_KERNEL);
> +
> + return lpg->lut_bitmap ? 0 : -ENOMEM;
> +}
> +
> +static int lpg_parse_dtest(struct lpg *lpg)
> +{
> + struct lpg_channel *chan;
> + struct device_node *np = lpg->dev->of_node;
> + int count;
> + int ret;
> + int i;
> +
> + count = of_property_count_u32_elems(np, "qcom,dtest");
> + if (count == -EINVAL) {
> + return 0;
> + } else if (count < 0 || count != lpg->data->num_channels * 2) {
> + ret = count;
> + goto err_malformed;
> + }
> +
> + for (i = 0; i < lpg->data->num_channels; i++) {
> + chan = &lpg->channels[i];
> +
> + ret = of_property_read_u32_index(np, "qcom,dtest", i * 2,
> + &chan->dtest_line);
> + if (ret)
> + goto err_malformed;
> +
> + ret = of_property_read_u32_index(np, "qcom,dtest", i * 2 + 1,
> + &chan->dtest_value);
> + if (ret)
> + goto err_malformed;
> + }
> +
> + return 0;
> +
> +err_malformed:
> + dev_err(lpg->dev, "malformed qcom,dtest\n");
> + return ret;
> +}
> +
> +static int lpg_probe(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + struct lpg *lpg;
> + int ret;
> + int i;
> +
> + lpg = devm_kzalloc(&pdev->dev, sizeof(*lpg), GFP_KERNEL);
> + if (!lpg)
> + return -ENOMEM;
> +
> + lpg->data = of_device_get_match_data(&pdev->dev);
> + if (!lpg->data)
> + return -EINVAL;
> +
> + lpg->dev = &pdev->dev;
> +
> + lpg->map = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!lpg->map) {
> + dev_err(&pdev->dev, "parent regmap unavailable\n");
> + return -ENXIO;
> + }
> +
> + ret = lpg_init_channels(lpg);
> + if (ret < 0)
> + return ret;
> +
> + ret = lpg_init_triled(lpg);
> + if (ret < 0)
> + return ret;
> +
> + ret = lpg_init_lut(lpg);
> + if (ret < 0)
> + return ret;
> +
> + ret = lpg_parse_dtest(lpg);
> + if (ret < 0)
> + return ret;
> +
> + for_each_available_child_of_node(pdev->dev.of_node, np) {
> + ret = lpg_add_led(lpg, np);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < lpg->num_channels; i++)
> + lpg_apply_dtest(&lpg->channels[i]);
> +
> + ret = lpg_add_pwm(lpg);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, lpg);
> +
> + return 0;
> +}
> +
> +static int lpg_remove(struct platform_device *pdev)
> +{
> + struct lpg *lpg = platform_get_drvdata(pdev);
> +
> + pwmchip_remove(&lpg->pwm);
> +
> + return 0;
> +}
> +
> +static const struct lpg_data pm8916_pwm_data = {
> + .pwm_9bit_mask = BIT(2),
> +
> + .num_channels = 1,
> + .channels = (struct lpg_channel_data[]) {
> + { .base = 0xbc00 },
> + },
> +};
> +
> +static const struct lpg_data pm8941_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 64,
> +
> + .triled_base = 0xd000,
> +
> + .pwm_9bit_mask = 3 << 4,
> +
> + .num_channels = 8,
> + .channels = (struct lpg_channel_data[]) {
> + { .base = 0xb100 },
> + { .base = 0xb200 },
> + { .base = 0xb300 },
> + { .base = 0xb400 },
> + { .base = 0xb500, .triled_mask = BIT(5) },
> + { .base = 0xb600, .triled_mask = BIT(6) },
> + { .base = 0xb700, .triled_mask = BIT(7) },
> + { .base = 0xb800 },
> + },
> +};
> +
> +static const struct lpg_data pm8994_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 64,
> +
> + .pwm_9bit_mask = 3 << 4,
> +
> + .num_channels = 6,
> + .channels = (struct lpg_channel_data[]) {
> + { .base = 0xb100 },
> + { .base = 0xb200 },
> + { .base = 0xb300 },
> + { .base = 0xb400 },
> + { .base = 0xb500 },
> + { .base = 0xb600 },
> + },
> +};
> +
> +static const struct lpg_data pmi8994_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 24,
> +
> + .triled_base = 0xd000,
> +
> + .pwm_9bit_mask = BIT(4),
> +
> + .num_channels = 4,
> + .channels = (struct lpg_channel_data[]) {
> + { .base = 0xb100, .triled_mask = BIT(5) },
> + { .base = 0xb200, .triled_mask = BIT(6) },
> + { .base = 0xb300, .triled_mask = BIT(7) },
> + { .base = 0xb400 },
> + },
> +};
> +
> +static const struct lpg_data pmi8998_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 49,
> +
> + .pwm_9bit_mask = BIT(4),
> +
> + .num_channels = 6,
> + .channels = (struct lpg_channel_data[]) {
> + { .base = 0xb100 },
> + { .base = 0xb200 },
> + { .base = 0xb300, .triled_mask = BIT(5) },
> + { .base = 0xb400, .triled_mask = BIT(6) },
> + { .base = 0xb500, .triled_mask = BIT(7) },
> + { .base = 0xb600 },
> + },
> +};
> +
> +static const struct of_device_id lpg_of_table[] = {
> + { .compatible = "qcom,pm8916-pwm", .data = &pm8916_pwm_data },
> + { .compatible = "qcom,pm8941-lpg", .data = &pm8941_lpg_data },
> + { .compatible = "qcom,pm8994-lpg", .data = &pm8994_lpg_data },
> + { .compatible = "qcom,pmi8994-lpg", .data = &pmi8994_lpg_data },
> + { .compatible = "qcom,pmi8998-lpg", .data = &pmi8998_lpg_data },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, lpg_of_table);
> +
> +static struct platform_driver lpg_driver = {
> + .probe = lpg_probe,
> + .remove = lpg_remove,
> + .driver = {
> + .name = "qcom-spmi-lpg",
> + .of_match_table = lpg_of_table,
> + },
> +};
> +module_platform_driver(lpg_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm TRI LED driver");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 1/5] checks: add a string checks for label, bootargs and stdout-path
From: David Gibson @ 2017-11-20 0:09 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171117144515.10870-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
On Fri, Nov 17, 2017 at 08:45:11AM -0600, Rob Herring wrote:
> Add more string property checks for label, bootargs, and stdout-path.
Where does 'label' appear? I'm not immediately recalling it as a
property with global meaning.
bootargs and stdout-path are from /chosen of course. I have some
mixed feelings about whether it's reasonable to check it's a string
everywhere, rather than specifically just in /chosen.
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> checks.c | 4 ++++
> tests/bad-string-props.dts | 3 +++
> tests/run_tests.sh | 2 +-
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/checks.c b/checks.c
> index f5bf5f97a3ad..a4a9d37ca19b 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -586,6 +586,9 @@ WARNING_IF_NOT_CELL(interrupt_cells_is_cell, "#interrupt-cells");
> WARNING_IF_NOT_STRING(device_type_is_string, "device_type");
> WARNING_IF_NOT_STRING(model_is_string, "model");
> WARNING_IF_NOT_STRING(status_is_string, "status");
> +WARNING_IF_NOT_STRING(label_is_string, "label");
> +WARNING_IF_NOT_STRING(bootargs_is_string, "bootargs");
> +WARNING_IF_NOT_STRING(stdout_path_is_string, "stdout-path");
>
> static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
> struct node *node)
> @@ -1236,6 +1239,7 @@ static struct check *check_table[] = {
>
> &address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell,
> &device_type_is_string, &model_is_string, &status_is_string,
> + &label_is_string, &bootargs_is_string, &stdout_path_is_string,
>
> &property_name_chars_strict,
> &node_name_chars_strict,
> diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
> index 396f82069cf7..9b5a7a1736ee 100644
> --- a/tests/bad-string-props.dts
> +++ b/tests/bad-string-props.dts
> @@ -4,4 +4,7 @@
> device_type = <0xdeadbeef>;
> model = <0xdeadbeef>;
> status = <0xdeadbeef>;
> + bootargs = <0xdeadbeef>;
> + stdout-path = <0xdeadbeef>;
> + label = <0xdeadbeef>;
> };
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 850bc165e757..c610aaeb053e 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -546,7 +546,7 @@ dtc_tests () {
> check_tests bad-name-property.dts name_properties
>
> check_tests bad-ncells.dts address_cells_is_cell size_cells_is_cell interrupt_cells_is_cell
> - check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string
> + check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string bootargs_is_string stdout_path_is_string label_is_string
> check_tests bad-reg-ranges.dts reg_format ranges_format
> check_tests bad-empty-ranges.dts ranges_format
> check_tests reg-ranges-root.dts reg_format ranges_format
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] checks: add string list check
From: David Gibson @ 2017-11-20 0:10 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171117144515.10870-3-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4028 bytes --]
On Fri, Nov 17, 2017 at 08:45:12AM -0600, Rob Herring wrote:
> Add a check for string list properties with compatible being the first
> check.
>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Not applying right now, while resolving queries on 1/5.
> ---
> checks.c | 34 ++++++++++++++++++++++++++++++++++
> tests/bad-string-props.dts | 8 ++++++++
> tests/run_tests.sh | 2 +-
> 3 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/checks.c b/checks.c
> index a4a9d37ca19b..4e23f29486bb 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -179,6 +179,36 @@ static void check_is_string(struct check *c, struct dt_info *dti,
> #define ERROR_IF_NOT_STRING(nm, propname) \
> ERROR(nm, check_is_string, (propname))
>
> +static void check_is_string_list(struct check *c, struct dt_info *dti,
> + struct node *node)
> +{
> + int rem, l;
> + struct property *prop;
> + char *propname = c->data;
> + char *str;
> +
> + prop = get_property(node, propname);
> + if (!prop)
> + return; /* Not present, assumed ok */
> +
> + str = prop->val.val;
> + rem = prop->val.len;
> + while (rem > 0) {
> + l = strnlen(str, rem);
> + if (l == rem) {
> + FAIL(c, dti, "\"%s\" property in %s is not a string list",
> + propname, node->fullpath);
> + break;
> + }
> + rem -= l + 1;
> + str += l + 1;
> + }
> +}
> +#define WARNING_IF_NOT_STRING_LIST(nm, propname) \
> + WARNING(nm, check_is_string_list, (propname))
> +#define ERROR_IF_NOT_STRING_LIST(nm, propname) \
> + ERROR(nm, check_is_string_list, (propname))
> +
> static void check_is_cell(struct check *c, struct dt_info *dti,
> struct node *node)
> {
> @@ -590,6 +620,8 @@ WARNING_IF_NOT_STRING(label_is_string, "label");
> WARNING_IF_NOT_STRING(bootargs_is_string, "bootargs");
> WARNING_IF_NOT_STRING(stdout_path_is_string, "stdout-path");
>
> +WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
> +
> static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
> struct node *node)
> {
> @@ -1241,6 +1273,8 @@ static struct check *check_table[] = {
> &device_type_is_string, &model_is_string, &status_is_string,
> &label_is_string, &bootargs_is_string, &stdout_path_is_string,
>
> + &compatible_is_string_list,
> +
> &property_name_chars_strict,
> &node_name_chars_strict,
>
> diff --git a/tests/bad-string-props.dts b/tests/bad-string-props.dts
> index 9b5a7a1736ee..5e226ce0c736 100644
> --- a/tests/bad-string-props.dts
> +++ b/tests/bad-string-props.dts
> @@ -7,4 +7,12 @@
> bootargs = <0xdeadbeef>;
> stdout-path = <0xdeadbeef>;
> label = <0xdeadbeef>;
> +
> + node1 {
> + compatible = <0xdeadbeef>;
> + };
> +
> + node2 {
> + compatible = "good", <0xdeadbeef>;
> + };
> };
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index c610aaeb053e..f492cf78ae84 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -546,7 +546,7 @@ dtc_tests () {
> check_tests bad-name-property.dts name_properties
>
> check_tests bad-ncells.dts address_cells_is_cell size_cells_is_cell interrupt_cells_is_cell
> - check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string bootargs_is_string stdout_path_is_string label_is_string
> + check_tests bad-string-props.dts device_type_is_string model_is_string status_is_string bootargs_is_string stdout_path_is_string label_is_string compatible_is_string_list
> check_tests bad-reg-ranges.dts reg_format ranges_format
> check_tests bad-empty-ranges.dts ranges_format
> check_tests reg-ranges-root.dts reg_format ranges_format
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] checks: add string list check for *-names properties
From: David Gibson @ 2017-11-20 0:10 UTC (permalink / raw)
To: Andre Przywara
Cc: Rob Herring, devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8eea3f9e-5512-dc6c-f755-58cc333db732-5wv7dgnIgG8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
On Fri, Nov 17, 2017 at 03:12:02PM +0000, Andre Przywara wrote:
> Hi,
>
> On 17/11/17 14:45, Rob Herring wrote:
> > Add a string list check for common properties ending in "-names" such as
> > reg-names or interrupt-names.
> >
> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> > checks.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/checks.c b/checks.c
> > index 4e23f29486bb..346b0256f9cb 100644
> > --- a/checks.c
> > +++ b/checks.c
> > @@ -622,6 +622,21 @@ WARNING_IF_NOT_STRING(stdout_path_is_string, "stdout-path");
> >
> > WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
> >
> > +static void check_names_is_string_list(struct check *c, struct dt_info *dti,
> > + struct node *node)
> > +{
> > + struct property *prop;
> > +
> > + for_each_property(node, prop) {
> > + if (!strstr(prop->name, "-names"))
>
> But that would match "actually-names-dont-matter" as well, resulting in
> a false positive? Should we check if the string appears at the *end* of
> the property name?
Yes, we should.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] checks: add string list check for *-names properties
From: David Gibson @ 2017-11-20 0:11 UTC (permalink / raw)
To: Rob Herring
Cc: Andre Przywara, Devicetree Compiler,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqLQwZ=grCDwjUORaE0NnmC_qJBBtJtT7X0-Z2BGujcfmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]
On Fri, Nov 17, 2017 at 12:07:48PM -0600, Rob Herring wrote:
> On Fri, Nov 17, 2017 at 9:12 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
> > Hi,
> >
> > On 17/11/17 14:45, Rob Herring wrote:
> >> Add a string list check for common properties ending in "-names" such as
> >> reg-names or interrupt-names.
> >>
> >> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> ---
> >> checks.c | 17 ++++++++++++++++-
> >> 1 file changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/checks.c b/checks.c
> >> index 4e23f29486bb..346b0256f9cb 100644
> >> --- a/checks.c
> >> +++ b/checks.c
> >> @@ -622,6 +622,21 @@ WARNING_IF_NOT_STRING(stdout_path_is_string, "stdout-path");
> >>
> >> WARNING_IF_NOT_STRING_LIST(compatible_is_string_list, "compatible");
> >>
> >> +static void check_names_is_string_list(struct check *c, struct dt_info *dti,
> >> + struct node *node)
> >> +{
> >> + struct property *prop;
> >> +
> >> + for_each_property(node, prop) {
> >> + if (!strstr(prop->name, "-names"))
> >
> > But that would match "actually-names-dont-matter" as well, resulting in
> > a false positive? Should we check if the string appears at the *end* of
> > the property name?
>
> Perhaps. IMO, once a word is used, it needs to be reserved for that
> purpose. For example, the gpio hogs binding use of "gpios" with just
> numbers and no phandle is bad because we have a mixture of types for a
> given property name or suffix. So we should really enforce that
> "-names" only appears as a suffix and use anywhere else is a warning.
That sounds... overly restrictive to me. The grabbing of a whole
bunch of gpios words is an example of poor - or at least nonstandard -
binding design IMO.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] checks: check for #{size,address}-cells without child nodes
From: David Gibson @ 2017-11-20 0:14 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171117144515.10870-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]
On Fri, Nov 17, 2017 at 08:45:14AM -0600, Rob Herring wrote:
> nodes with a 'reg' property nor a ranges property.
>
> An exception may be an overlay that adds nodes, but this case would
> need
Sentence doesn't seem finished..
In any case, I'm not sure this is a good idea. It's not uncommon to
have bus bridge nodes that ought to have a well defined #address and
#size cells, but just don't happen to have any plugged devices yet.
An overlay that adds nodes is one possibility, but a bus where the
children can be probed is another.
The check for 'ranges' will get some of those cases, but a bus bridge
which doesn't directly map the child address space into the parents
(e.g. indirect access) is perfectly legitimate still.
>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> checks.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/checks.c b/checks.c
> index 346b0256f9cb..a785b81bea07 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -982,6 +982,31 @@ static void check_avoid_default_addr_size(struct check *c, struct dt_info *dti,
> WARNING(avoid_default_addr_size, check_avoid_default_addr_size, NULL,
> &addr_size_cells);
>
> +static void check_avoid_unecessary_addr_size(struct check *c, struct dt_info *dti,
> + struct node *node)
> +{
> + struct property *prop;
> + struct node *child;
> + bool has_reg = false;
> +
> + if (!node->parent || node->addr_cells < 0 || node->size_cells < 0)
> + return;
> +
> + if (get_property(node, "ranges") || !node->children)
> + return;
> +
> + for_each_child(node, child) {
> + prop = get_property(child, "reg");
> + if (prop)
> + has_reg = true;
> + }
> +
> + if (!has_reg)
> + FAIL(c, dti, "unnecessary #address-cells/#size-cells without \"ranges\" or child \"reg\" property in %s",
> + node->fullpath);
> +}
> +WARNING(avoid_unecessary_addr_size, check_avoid_unecessary_addr_size, NULL, &avoid_default_addr_size);
> +
> static void check_obsolete_chosen_interrupt_controller(struct check *c,
> struct dt_info *dti,
> struct node *node)
> @@ -1306,6 +1331,7 @@ static struct check *check_table[] = {
> &simple_bus_reg,
>
> &avoid_default_addr_size,
> + &avoid_unecessary_addr_size,
> &obsolete_chosen_interrupt_controller,
>
> &clocks_property,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] checks: add aliases node checks
From: David Gibson @ 2017-11-20 0:24 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171117144515.10870-6-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]
On Fri, Nov 17, 2017 at 08:45:15AM -0600, Rob Herring wrote:
> Add checks for aliases node that properties are a valid path and that the
> alias property names are a known, standard name.
>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
The check for valid paths is definitely good. I'm not convinced by
the restrictive list of alias names though.
It's true that in trees originating from dts files, aliases don't have
a lot of purpose, but in trees from real OF that's not so. It's
pretty reasonable for those to define arbitrary aliases for user
convenience - and the names here aren't even the common ones for that
purpose: those are "net", "disk" and "cd" so you can do things like
"boot disk" from the user interface without having to enter a hideous
path giving PCI and SCSI addresses.
> ---
> checks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/checks.c b/checks.c
> index a785b81bea07..2c5b6c2eacb3 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -637,6 +637,48 @@ static void check_names_is_string_list(struct check *c, struct dt_info *dti,
> }
> WARNING(names_is_string_list, check_names_is_string_list, NULL);
>
> +static void check_alias_paths(struct check *c, struct dt_info *dti,
> + struct node *node)
> +{
> + struct property *prop;
> +
> + if (!streq(node->name, "aliases"))
> + return;
> +
> + for_each_property(node, prop) {
> + if (!prop->val.val || !get_node_by_path(dti->dt, prop->val.val))
> + FAIL(c, dti, "aliases property '%s' is not a valid node (%s)",
> + prop->name, prop->val.val);
> + }
> +}
> +WARNING(alias_paths, check_alias_paths, NULL);
> +
> +static void check_known_aliases(struct check *c, struct dt_info *dti,
> + struct node *node)
> +{
> + int i;
> + struct property *prop;
> + static char *aliases_strings[] = {
> + "ethernet", "gpio", "i2c", "rtc", "serial", "spi"
> + };
> +
> + if (!streq(node->name, "aliases"))
> + return;
> +
> + for_each_property(node, prop) {
> + for (i = 0; i < ARRAY_SIZE(aliases_strings); i++) {
> + if (strstarts(prop->name, aliases_strings[i]))
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(aliases_strings)) {
> + FAIL(c, dti, "unknown alias name %s", prop->name);
> + continue;
> + }
> + }
> +}
> +WARNING(known_aliases, check_known_aliases, NULL);
> +
> static void fixup_addr_size_cells(struct check *c, struct dt_info *dti,
> struct node *node)
> {
> @@ -1355,6 +1397,8 @@ static struct check *check_table[] = {
> &gpios_property,
> &interrupts_property,
>
> + &known_aliases, &alias_paths,
> +
> &always_fail,
> };
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 3/4] pmbus (core): Add virtual page config bit
From: Andrew Jeffery @ 2017-11-20 0:40 UTC (permalink / raw)
To: Guenter Roeck, linux-hwmon-u79uwXL29TY76Z2rM5mHXA
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, joel-U3u1mxZcP9KHXe+LvDLADg,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <3e84eca8-7a10-abd3-b8df-babb5f4ac8a0-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3197 bytes --]
On Sun, 2017-11-19 at 08:49 -0800, Guenter Roeck wrote:
> On 11/17/2017 07:26 PM, Andrew Jeffery wrote:
> > Some circumstances call for virtual pages, to expose multiple values
> > packed into an extended PMBus register in a manner non-compliant with
> > the PMBus standard. An example of this is the Maxim MAX31785 controller, which
> > extends the READ_FAN_SPEED_1 PMBus register from two to four bytes to support
> > tach readings for both rotors of a dual rotor fan. This extended register
> > contains two word-sized values, one reporting the rate of the fastest rotor,
> > the other the rate of the slowest. The concept of virtual pages aids this
> > situation by mapping the page number onto the value to be selected from the
> > vectored result.
> >
> > We should not try to set virtual pages on the device as such a page explicitly
> > doesn't exist; add a flag so we can avoid doing so.
> >
> > Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> > ---
> > drivers/hwmon/pmbus/pmbus.h | 2 ++
> > drivers/hwmon/pmbus/pmbus_core.c | 10 +++++++---
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index b54d7604d3ef..d39d506aa63e 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -372,6 +372,8 @@ enum pmbus_sensor_classes {
> > #define PMBUS_HAVE_PWM12 BIT(20)
> > #define PMBUS_HAVE_PWM34 BIT(21)
> >
> > +#define PMBUS_PAGE_VIRTUAL BIT(31)
> > +
> > enum pmbus_data_format { linear = 0, direct, vid };
> > enum vrm_version { vr11 = 0, vr12, vr13 };
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index 631db88526b6..ba97230fcde7 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -164,14 +164,18 @@ int pmbus_set_page(struct i2c_client *client, int page)
> > int rv = 0;
> > int newpage;
> >
> > - if (page >= 0 && page != data->currpage) {
> > + if (page < 0 || page == data->currpage)
> > + return 0;
> > +
> > + if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL)) {
> > rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> > if (newpage != page)
> > rv = -EIO;
>
> This should probably be something like
> rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> if (rv < 0)
> return rv;
> newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
> if (newpage < 0)
> return newpage;
> if (newpage != page)
> return -EIO;
>
> We can actually drop 'newpage' and just use 'rv'.
>
> > - else
> > - data->currpage = page;
> > }
> > +
> > + data->currpage = page;
> > +
>
> ... otherwise currpage is set even on error.
Thanks for catching that, clearly I needed to pay more attention
resolving the conflicts when rebasing on hwmon-next. I'll address all
your points.
Cheers,
Andrew
>
> > return rv;
>
> this can then be
> return 0;
>
> > }
> > EXPORT_SYMBOL_GPL(pmbus_set_page);
> >
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: RFC: Copying Device Tree File into reserved area of VMLINUX before deployment
From: Frank Rowand @ 2017-11-20 4:23 UTC (permalink / raw)
To: linux-kernel, LKML, devicetree@vger.kernel.org, Rob Herring,
Frank Rowand
In-Reply-To: <0b31e22b-202f-6fca-28f2-e16e6af6c6b7@emagii.com>
adding devicetree list, devicetree maintainers
On 11/18/17 12:59, Ulf Samuelsson wrote:
> I noticed when checking out the OpenWRT support for the board that they have a method to avoid having to pass the device tree address to the kernel, and can thus boot device tree based kernels with U-boots that
> does not support device trees.
>
> Is this something that would be considered useful for including in mainstream:
>
> BACKGROUND:
> Trying to load a yocto kernel into a MIPS target (MT7620A based),
> and the U-Boot is more than stupid.
> Does not support the "run" command as an example.
> They modified the U-Boot MAGIC Word to complicate things.
> The U-Boot is not configured to use device tree files.
> The board runs a 2.6 kernel right now.
>
> Several attempts by me a and others to rebuild U-Boot according to the H/W vendors source code and build instructions results in a bricked unit. Bricked units cannot be recovered.
>
> Not my choice of H/W, so I cannot change it.
>
>
> ===================================================================
> OPENWRT:
> I noticed when checking out the OpenWRT support for the board that they have a method to avoid having to pass the device tree address to the kernel, and can thus boot device tree based kernels with U-boots that
> does not support device trees.
>
> What they do is to reserve 16 kB of kernel space, and tag it with an ASCII string "OWRTDTB:".
> After the kernel and dtb is built, a utility "patch-dtb" will update the vmlinux binary, copying in the device tree file.
>
> ===================================================================
> It would be useful to me, and I could of course patch the mainstream kernel, but first I would like to check if this is of interest for mainstream.
>
> I envisage the support would look something like:
>
> ============
> Kconfig.
> config MIPS
> select HAVE_IMAGE_DTB
>
> config HAVE_IMAGE_DTB
> bool
>
> if HAVE_IMAGE_DTB
> config IMAGE_DTB
> bool "Allocated space for DTB within image
>
> config DTB_SIZE
> int "DTB space (kB)
>
> config DTB_TAG
> string "DTB space tag"
> default "OWRTDTB:"
> endif
>
> ============
> Some Makefile
> obj-$(CONFIG_INCLUDE_DTB) += image_dtb.o
>
> ============
> image_dtb.S:
> .text
> .align 5
> .ascii CONFIG_DTB_TAG
> EXPORT(__image_dtb)
> .fill DTB_SIZE * 1024
>
> ===================
> arch/mips/xxx/of.c:
>
> #if defined(CONFIG_IMAGE_DTB)
> if (<conditions to boot from dtb_space>)
> __dt_setup_arch(__dtb_start);
> else
> __dt_setup_arch(&__image_dtb);
> #else
> __dt_setup_arch(__dtb_start);
> #endif
>
> I imagine that if the support is enabled for a target, it should
> be possible to override it with a CMDLINE argument
>
>
> They do something similar for the CMDLINE; copying it into the vmlinux, to allow a smaller boot
>
>
>
^ permalink raw reply
* Re: RFC: Copying Device Tree File into reserved area of VMLINUX before deployment
From: Frank Rowand @ 2017-11-20 4:32 UTC (permalink / raw)
To: linux-kernel-AoFPY8dbyRPQT0dZR+AlfA, LKML,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring
In-Reply-To: <a0f9d598-04af-fddd-58ad-09db10ec19da-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Ulf,
On 11/19/17 23:23, Frank Rowand wrote:
> adding devicetree list, devicetree maintainers
>
> On 11/18/17 12:59, Ulf Samuelsson wrote:
>> I noticed when checking out the OpenWRT support for the board that they have a method to avoid having to pass the device tree address to the kernel, and can thus boot device tree based kernels with U-boots that
>> does not support device trees.
>>
>> Is this something that would be considered useful for including in mainstream:
>>
>> BACKGROUND:
>> Trying to load a yocto kernel into a MIPS target (MT7620A based),
>> and the U-Boot is more than stupid.
>> Does not support the "run" command as an example.
>> They modified the U-Boot MAGIC Word to complicate things.
>> The U-Boot is not configured to use device tree files.
>> The board runs a 2.6 kernel right now.
>>
>> Several attempts by me a and others to rebuild U-Boot according to
>> the H/W vendors source code and build instructions results in a
>> bricked unit. Bricked units cannot be recovered.
Hopefully you have brought this to the attention of the vendor. U-Boot
is GPL v2 (or in some ways possibly GPL v2 or later), so if you can not
build U-Boot that is equivalent to the binary U-Boot they shipped, the
vendor may want to ensure that they are shipping the proper source and
build instructions.
>> Not my choice of H/W, so I cannot change it.
>>
>>
>> ===================================================================
>> OPENWRT:
>> I noticed when checking out the OpenWRT support for the board that
>> they have a method to avoid having to pass the device tree address
>> to the kernel, and can thus boot device tree based kernels with
>> U-boots that does not support device trees.
>>
>> What they do is to reserve 16 kB of kernel space, and tag it with
>> an ASCII string "OWRTDTB:". After the kernel and dtb is built, a
>> utility "patch-dtb" will update the vmlinux binary, copying in the
>> device tree file.
>>
>> ===================================================================
>> It would be useful to me, and I could of course patch the
>> mainstream kernel, but first I would like to check if this is of
>> interest for mainstream.
Not in this form. Hard coding a fixed size area in the boot image
to contain the FDT (aka DTB) is a non-starter.
And again, I would first approach the H/W vendor before trying to
come up with a work around like this.
>> I envisage the support would look something like:
>>
>> ============
>> Kconfig.
>> config MIPS
>> select HAVE_IMAGE_DTB
>>
>> config HAVE_IMAGE_DTB
>> bool
>>
>> if HAVE_IMAGE_DTB
>> config IMAGE_DTB
>> bool "Allocated space for DTB within image
>>
>> config DTB_SIZE
>> int "DTB space (kB)
>>
>> config DTB_TAG
>> string "DTB space tag"
>> default "OWRTDTB:"
>> endif
>>
>> ============
>> Some Makefile
>> obj-$(CONFIG_INCLUDE_DTB) += image_dtb.o
>>
>> ============
>> image_dtb.S:
>> .text
>> .align 5
>> .ascii CONFIG_DTB_TAG
>> EXPORT(__image_dtb)
>> .fill DTB_SIZE * 1024
>>
>> ===================
>> arch/mips/xxx/of.c:
>>
>> #if defined(CONFIG_IMAGE_DTB)
>> if (<conditions to boot from dtb_space>)
>> __dt_setup_arch(__dtb_start);
>> else
>> __dt_setup_arch(&__image_dtb);
>> #else
>> __dt_setup_arch(__dtb_start);
>> #endif
>>
>> I imagine that if the support is enabled for a target, it should
>> be possible to override it with a CMDLINE argument
>>
>>
>> They do something similar for the CMDLINE; copying it into the vmlinux, to allow a smaller boot
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v6 0/4] pmbus: Expand fan support and add MAX31785 driver
From: Andrew Jeffery @ 2017-11-20 4:42 UTC (permalink / raw)
To: linux-hwmon
Cc: Andrew Jeffery, linux, jdelvare, corbet, devicetree, linux-kernel,
linux-doc, joel, openbmc
Hello,
This series introduces support for the MAX31785 intelligent fan controller, a
PMBus device providing closed-loop fan control among a number of other
features. Along the way the series adds support to control fans and create
virtual pages to the PMBus core, the latter to support some of the more
annoying design decisions found in the 'A' variant of the chip.
This is the sixth spin of the series. v5 can be found here:
https://lkml.org/lkml/2017/11/17/910
v6 fixes an error-handling bug I introduced in pmbus_set_page() with PATCH 3/4,
as pointed out by Guenter on v5[1]. The rework implements his suggested
solution.
A few other other minor fixes are included:
* PATCH 1/4:
* Avoid potential NULL dereference in pmbus_get_fan_rate(). It
amounts to being an error path that should never happen in practice, but
lets make sure we don't die if we do hit it.
* Drop a redundant test for the presence of FAN_COMMAND_x in
pmbus_add_fan_ctrl(); document that its presence is a precondition to
calling it (this was already tested at the one call-site).
* PATCH 2/4: Avoid scaling an error code in pmbus_pwm_enable(). Rather, test
the result of pmbus_get_fan_rate_cached() before performing the scaling when
switching to PWM mode.
[1] https://lkml.org/lkml/2017/11/19/280
Please review!
Andrew
Andrew Jeffery (4):
pmbus (core): Add fan control support
pmbus (max31785): Add fan control
pmbus (core): Add virtual page config bit
pmbus (max31785): Add dual tachometer support
Documentation/hwmon/max31785 | 15 +-
drivers/hwmon/pmbus/max31785.c | 285 +++++++++++++++++++++++++++++++-
drivers/hwmon/pmbus/pmbus.h | 41 ++++-
drivers/hwmon/pmbus/pmbus_core.c | 265 +++++++++++++++++++++++++++---
4 files changed, 575 insertions(+), 31 deletions(-)
base-commit: ded0eb83449e8fcba22fd2736826336e101ffbcb
--
git-series 0.9.1
^ permalink raw reply
* [PATCH v6 1/4] pmbus (core): Add fan control support
From: Andrew Jeffery @ 2017-11-20 4:42 UTC (permalink / raw)
To: linux-hwmon
Cc: Andrew Jeffery, linux, jdelvare, corbet, devicetree, linux-kernel,
linux-doc, joel, openbmc
In-Reply-To: <cover.dfb0a63462b38b9384b1d077fd62d17a11a7a0dd.1511152748.git-series.andrew@aj.id.au>
Expose fanX_target, pwmX and pwmX_enable hwmon sysfs attributes.
Fans in a PMBus device are driven by the configuration of two registers,
FAN_CONFIG_x_y and FAN_COMMAND_x: FAN_CONFIG_x_y dictates how the fan
and the tacho operate (if installed), while FAN_COMMAND_x sets the
desired fan rate. The unit of FAN_COMMAND_x is dependent on the
operational fan mode, RPM or PWM percent duty, as determined by the
corresponding configuration in FAN_CONFIG_x_y.
The mapping of fanX_target, pwmX and pwmX_enable onto FAN_CONFIG_x_y and
FAN_COMMAND_x is implemented with the addition of virtual registers to
facilitate the necessary side-effects of each access:
1. PMBUS_VIRT_FAN_TARGET_x
2. PMBUS_VIRT_PWM_x
3. PMBUS_VIRT_PWM_ENABLE_x
Some complexity arises with the fanX_target and pwmX attributes both mapping
onto FAN_COMMAND_x: There is no general mapping between PWM percent duty and
RPM, so we can't display values in either attribute in terms of the other
(which in my mind is the intuitive, if impossible, behaviour). This problem
also affects the pwmX_enable attribute which allows userspace to switch between
full speed, manual PWM and a number of automatic control modes, possibly
including a switch to RPM behaviour (e.g. automatically adjusting PWM duty to
reach a RPM target, the behaviour of fanX_target).
The next most intuitive behaviour is for fanX_target and pwmX to simply be
independent, to retain their most recently set value even if that value is not
active on the hardware (due to switching to the alternative control mode). This
property of retaining the value independent of the hardware state has useful
results for both userspace and the kernel: Userspace always sees a sensible
value in the attribute (the last thing it was set to, as opposed to 0 or
receiving an error on read), and the kernel can use the attributes as a value
cache. This latter point eases the implementation of pwmX_enable, which can
look up the associated pmbus_sensor object, take its cached value and apply it
to hardware on changing control mode. This ensures we will not arbitrarily set
a PWM value as an RPM value or vice versa, and we can assume that the RPM or
PWM value set was sensible at least at some point in the past.
Finally, the DIRECT mode coefficients of some controllers is different between
RPM and PWM percent duty control modes, so PSC_PWM is introduced to capture the
necessary coefficients. As pmbus core had no PWM support previously PSC_FAN
continues to be used to capture the RPM DIRECT coefficients, but in order to
avoid falsely applying RPM scaling to PWM values I have introduced the
PMBUS_HAVE_PWM12 and PMB_BUS_HAVE_PWM34 feature bits. These feature bits allow
drivers to explicitly declare PWM support in order to have the attributes
exposed.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/pmbus.h | 39 ++++-
drivers/hwmon/pmbus/pmbus_core.c | 238 +++++++++++++++++++++++++++++---
2 files changed, 259 insertions(+), 18 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index fa613bd209e3..b54d7604d3ef 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -190,6 +190,33 @@ enum pmbus_regs {
PMBUS_VIRT_VMON_UV_FAULT_LIMIT,
PMBUS_VIRT_VMON_OV_FAULT_LIMIT,
PMBUS_VIRT_STATUS_VMON,
+
+ /*
+ * RPM and PWM Fan control
+ *
+ * Drivers wanting to expose PWM control must define the behaviour of
+ * PMBUS_VIRT_PWM_[1-4] and PMBUS_VIRT_PWM_ENABLE_[1-4] in the
+ * {read,write}_word_data callback.
+ *
+ * pmbus core provides a default implementation for
+ * PMBUS_VIRT_FAN_TARGET_[1-4].
+ *
+ * TARGET, PWM and PWM_ENABLE members must be defined sequentially;
+ * pmbus core uses the difference between the provided register and
+ * it's _1 counterpart to calculate the FAN/PWM ID.
+ */
+ PMBUS_VIRT_FAN_TARGET_1,
+ PMBUS_VIRT_FAN_TARGET_2,
+ PMBUS_VIRT_FAN_TARGET_3,
+ PMBUS_VIRT_FAN_TARGET_4,
+ PMBUS_VIRT_PWM_1,
+ PMBUS_VIRT_PWM_2,
+ PMBUS_VIRT_PWM_3,
+ PMBUS_VIRT_PWM_4,
+ PMBUS_VIRT_PWM_ENABLE_1,
+ PMBUS_VIRT_PWM_ENABLE_2,
+ PMBUS_VIRT_PWM_ENABLE_3,
+ PMBUS_VIRT_PWM_ENABLE_4,
};
/*
@@ -223,6 +250,8 @@ enum pmbus_regs {
#define PB_FAN_1_RPM BIT(6)
#define PB_FAN_1_INSTALLED BIT(7)
+enum pmbus_fan_mode { percent = 0, rpm };
+
/*
* STATUS_BYTE, STATUS_WORD (lower)
*/
@@ -313,6 +342,7 @@ enum pmbus_sensor_classes {
PSC_POWER,
PSC_TEMPERATURE,
PSC_FAN,
+ PSC_PWM,
PSC_NUM_CLASSES /* Number of power sensor classes */
};
@@ -339,6 +369,8 @@ enum pmbus_sensor_classes {
#define PMBUS_HAVE_STATUS_FAN34 BIT(17)
#define PMBUS_HAVE_VMON BIT(18)
#define PMBUS_HAVE_STATUS_VMON BIT(19)
+#define PMBUS_HAVE_PWM12 BIT(20)
+#define PMBUS_HAVE_PWM34 BIT(21)
enum pmbus_data_format { linear = 0, direct, vid };
enum vrm_version { vr11 = 0, vr12, vr13 };
@@ -421,5 +453,10 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
int pmbus_do_remove(struct i2c_client *client);
const struct pmbus_driver_info *pmbus_get_driver_info(struct i2c_client
*client);
-
+int pmbus_get_fan_rate_device(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode);
+int pmbus_get_fan_rate_cached(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode);
+int pmbus_update_fan(struct i2c_client *client, int page, int id,
+ u8 config, u8 mask, u16 command);
#endif /* PMBUS_H */
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 52a58b8b6e1b..edc25efe7552 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -64,6 +64,7 @@ struct pmbus_sensor {
u16 reg; /* register */
enum pmbus_sensor_classes class; /* sensor class */
bool update; /* runtime sensor update needed */
+ bool convert; /* Whether or not to apply linear/vid/direct */
int data; /* Sensor data.
Negative if there was a read error */
};
@@ -128,6 +129,27 @@ struct pmbus_debugfs_entry {
u8 reg;
};
+static const int pmbus_fan_rpm_mask[] = {
+ PB_FAN_1_RPM,
+ PB_FAN_2_RPM,
+ PB_FAN_1_RPM,
+ PB_FAN_2_RPM,
+};
+
+static const int pmbus_fan_config_registers[] = {
+ PMBUS_FAN_CONFIG_12,
+ PMBUS_FAN_CONFIG_12,
+ PMBUS_FAN_CONFIG_34,
+ PMBUS_FAN_CONFIG_34
+};
+
+static const int pmbus_fan_command_registers[] = {
+ PMBUS_FAN_COMMAND_1,
+ PMBUS_FAN_COMMAND_2,
+ PMBUS_FAN_COMMAND_3,
+ PMBUS_FAN_COMMAND_4,
+};
+
void pmbus_clear_cache(struct i2c_client *client)
{
struct pmbus_data *data = i2c_get_clientdata(client);
@@ -197,6 +219,28 @@ int pmbus_write_word_data(struct i2c_client *client, int page, u8 reg,
}
EXPORT_SYMBOL_GPL(pmbus_write_word_data);
+
+static int pmbus_write_virt_reg(struct i2c_client *client, int page, int reg,
+ u16 word)
+{
+ int bit;
+ int id;
+ int rv;
+
+ switch (reg) {
+ case PMBUS_VIRT_FAN_TARGET_1 ... PMBUS_VIRT_FAN_TARGET_4:
+ id = reg - PMBUS_VIRT_FAN_TARGET_1;
+ bit = pmbus_fan_rpm_mask[id];
+ rv = pmbus_update_fan(client, page, id, bit, bit, word);
+ break;
+ default:
+ rv = -ENXIO;
+ break;
+ }
+
+ return rv;
+}
+
/*
* _pmbus_write_word_data() is similar to pmbus_write_word_data(), but checks if
* a device specific mapping function exists and calls it if necessary.
@@ -213,11 +257,38 @@ static int _pmbus_write_word_data(struct i2c_client *client, int page, int reg,
if (status != -ENODATA)
return status;
}
+
if (reg >= PMBUS_VIRT_BASE)
- return -ENXIO;
+ return pmbus_write_virt_reg(client, page, reg, word);
+
return pmbus_write_word_data(client, page, reg, word);
}
+int pmbus_update_fan(struct i2c_client *client, int page, int id,
+ u8 config, u8 mask, u16 command)
+{
+ int from;
+ int rv;
+ u8 to;
+
+ from = pmbus_read_byte_data(client, page,
+ pmbus_fan_config_registers[id]);
+ if (from < 0)
+ return from;
+
+ to = (from & ~mask) | (config & mask);
+ if (to != from) {
+ rv = pmbus_write_byte_data(client, page,
+ pmbus_fan_config_registers[id], to);
+ if (rv < 0)
+ return rv;
+ }
+
+ return _pmbus_write_word_data(client, page,
+ pmbus_fan_command_registers[id], command);
+}
+EXPORT_SYMBOL_GPL(pmbus_update_fan);
+
int pmbus_read_word_data(struct i2c_client *client, int page, u8 reg)
{
int rv;
@@ -230,6 +301,24 @@ int pmbus_read_word_data(struct i2c_client *client, int page, u8 reg)
}
EXPORT_SYMBOL_GPL(pmbus_read_word_data);
+static int pmbus_read_virt_reg(struct i2c_client *client, int page, int reg)
+{
+ int rv;
+ int id;
+
+ switch (reg) {
+ case PMBUS_VIRT_FAN_TARGET_1 ... PMBUS_VIRT_FAN_TARGET_4:
+ id = reg - PMBUS_VIRT_FAN_TARGET_1;
+ rv = pmbus_get_fan_rate_device(client, page, id, rpm);
+ break;
+ default:
+ rv = -ENXIO;
+ break;
+ }
+
+ return rv;
+}
+
/*
* _pmbus_read_word_data() is similar to pmbus_read_word_data(), but checks if
* a device specific mapping function exists and calls it if necessary.
@@ -245,8 +334,10 @@ static int _pmbus_read_word_data(struct i2c_client *client, int page, int reg)
if (status != -ENODATA)
return status;
}
+
if (reg >= PMBUS_VIRT_BASE)
- return -ENXIO;
+ return pmbus_read_virt_reg(client, page, reg);
+
return pmbus_read_word_data(client, page, reg);
}
@@ -311,6 +402,68 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)
return pmbus_read_byte_data(client, page, reg);
}
+static struct pmbus_sensor *pmbus_find_sensor(struct pmbus_data *data, int page,
+ int reg)
+{
+ struct pmbus_sensor *sensor;
+
+ for (sensor = data->sensors; sensor; sensor = sensor->next) {
+ if (sensor->page == page && sensor->reg == reg)
+ return sensor;
+ }
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int pmbus_get_fan_rate(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode,
+ bool from_cache)
+{
+ struct pmbus_data *data = i2c_get_clientdata(client);
+ bool want_rpm, have_rpm;
+ struct pmbus_sensor *s;
+ int config;
+ int reg;
+
+ want_rpm = (mode == rpm);
+
+ if (from_cache) {
+ reg = want_rpm ? PMBUS_VIRT_FAN_TARGET_1 : PMBUS_VIRT_PWM_1;
+ s = pmbus_find_sensor(data, page, reg + id);
+ if (IS_ERR(s))
+ return PTR_ERR(s);
+
+ return s->data;
+ }
+
+ config = pmbus_read_byte_data(client, page,
+ pmbus_fan_config_registers[id]);
+ if (config < 0)
+ return config;
+
+ have_rpm = !!(config & pmbus_fan_rpm_mask[id]);
+ if (want_rpm == have_rpm)
+ return pmbus_read_word_data(client, page,
+ pmbus_fan_command_registers[id]);
+
+ /* Can't sensibly map between RPM and PWM, just return zero */
+ return 0;
+}
+
+int pmbus_get_fan_rate_device(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode)
+{
+ return pmbus_get_fan_rate(client, page, id, mode, false);
+}
+EXPORT_SYMBOL_GPL(pmbus_get_fan_rate_device);
+
+int pmbus_get_fan_rate_cached(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode)
+{
+ return pmbus_get_fan_rate(client, page, id, mode, true);
+}
+EXPORT_SYMBOL_GPL(pmbus_get_fan_rate_cached);
+
static void pmbus_clear_fault_page(struct i2c_client *client, int page)
{
_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
@@ -512,7 +665,7 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
/* X = 1/m * (Y * 10^-R - b) */
R = -R;
/* scale result to milli-units for everything but fans */
- if (sensor->class != PSC_FAN) {
+ if (!(sensor->class == PSC_FAN || sensor->class == PSC_PWM)) {
R += 3;
b *= 1000;
}
@@ -566,6 +719,9 @@ static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
{
long val;
+ if (!sensor->convert)
+ return sensor->data;
+
switch (data->info->format[sensor->class]) {
case direct:
val = pmbus_reg2data_direct(data, sensor);
@@ -669,7 +825,7 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
}
/* Calculate Y = (m * X + b) * 10^R */
- if (sensor->class != PSC_FAN) {
+ if (!(sensor->class == PSC_FAN || sensor->class == PSC_PWM)) {
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
@@ -700,6 +856,9 @@ static u16 pmbus_data2reg(struct pmbus_data *data,
{
u16 regval;
+ if (!sensor->convert)
+ return val;
+
switch (data->info->format[sensor->class]) {
case direct:
regval = pmbus_data2reg_direct(data, sensor, val);
@@ -912,7 +1071,8 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
const char *name, const char *type,
int seq, int page, int reg,
enum pmbus_sensor_classes class,
- bool update, bool readonly)
+ bool update, bool readonly,
+ bool convert)
{
struct pmbus_sensor *sensor;
struct device_attribute *a;
@@ -922,12 +1082,18 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
return NULL;
a = &sensor->attribute;
- snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
- name, seq, type);
+ if (type)
+ snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
+ name, seq, type);
+ else
+ snprintf(sensor->name, sizeof(sensor->name), "%s%d",
+ name, seq);
+
sensor->page = page;
sensor->reg = reg;
sensor->class = class;
sensor->update = update;
+ sensor->convert = convert;
pmbus_dev_attr_init(a, sensor->name,
readonly ? S_IRUGO : S_IRUGO | S_IWUSR,
pmbus_show_sensor, pmbus_set_sensor);
@@ -1026,7 +1192,7 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,
curr = pmbus_add_sensor(data, name, l->attr, index,
page, l->reg, attr->class,
attr->update || l->update,
- false);
+ false, true);
if (!curr)
return -ENOMEM;
if (l->sbit && (info->func[page] & attr->sfunc)) {
@@ -1065,7 +1231,7 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,
return ret;
}
base = pmbus_add_sensor(data, name, "input", index, page, attr->reg,
- attr->class, true, true);
+ attr->class, true, true, true);
if (!base)
return -ENOMEM;
if (attr->sfunc) {
@@ -1589,13 +1755,6 @@ static const int pmbus_fan_registers[] = {
PMBUS_READ_FAN_SPEED_4
};
-static const int pmbus_fan_config_registers[] = {
- PMBUS_FAN_CONFIG_12,
- PMBUS_FAN_CONFIG_12,
- PMBUS_FAN_CONFIG_34,
- PMBUS_FAN_CONFIG_34
-};
-
static const int pmbus_fan_status_registers[] = {
PMBUS_STATUS_FAN_12,
PMBUS_STATUS_FAN_12,
@@ -1618,6 +1777,42 @@ static const u32 pmbus_fan_status_flags[] = {
};
/* Fans */
+
+/* Precondition: FAN_CONFIG_x_y and FAN_COMMAND_x must exist for the fan ID */
+static int pmbus_add_fan_ctrl(struct i2c_client *client,
+ struct pmbus_data *data, int index, int page, int id,
+ u8 config)
+{
+ struct pmbus_sensor *sensor;
+
+ sensor = pmbus_add_sensor(data, "fan", "target", index, page,
+ PMBUS_VIRT_FAN_TARGET_1 + id, PSC_FAN,
+ false, false, true);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ if (!((data->info->func[page] & PMBUS_HAVE_PWM12) ||
+ (data->info->func[page] & PMBUS_HAVE_PWM34)))
+ return 0;
+
+ sensor = pmbus_add_sensor(data, "pwm", NULL, index, page,
+ PMBUS_VIRT_PWM_1 + id, PSC_PWM,
+ false, false, true);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ sensor = pmbus_add_sensor(data, "pwm", "enable", index, page,
+ PMBUS_VIRT_PWM_ENABLE_1 + id, PSC_PWM,
+ true, false, false);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int pmbus_add_fan_attributes(struct i2c_client *client,
struct pmbus_data *data)
{
@@ -1652,9 +1847,18 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,
if (pmbus_add_sensor(data, "fan", "input", index,
page, pmbus_fan_registers[f],
- PSC_FAN, true, true) == NULL)
+ PSC_FAN, true, true, true) == NULL)
return -ENOMEM;
+ /* Fan control */
+ if (pmbus_check_word_register(client, page,
+ pmbus_fan_command_registers[f])) {
+ ret = pmbus_add_fan_ctrl(client, data, index,
+ page, f, regval);
+ if (ret < 0)
+ return ret;
+ }
+
/*
* Each fan status register covers multiple fans,
* so we have to do some magic.
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH v6 2/4] pmbus (max31785): Add fan control
From: Andrew Jeffery @ 2017-11-20 4:42 UTC (permalink / raw)
To: linux-hwmon
Cc: Andrew Jeffery, linux, jdelvare, corbet, devicetree, linux-kernel,
linux-doc, joel, openbmc
In-Reply-To: <cover.dfb0a63462b38b9384b1d077fd62d17a11a7a0dd.1511152748.git-series.andrew@aj.id.au>
The implementation makes use of the new fan control virtual registers exposed
by the pmbus core. It mixes use of the default implementations with some
overrides via the read/write handlers to handle FAN_COMMAND_1 on the MAX31785,
whose definition breaks the value range into various control bands dependent on
RPM or PWM mode.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/max31785 | 7 ++-
drivers/hwmon/pmbus/max31785.c | 138 +++++++++++++++++++++++++++++++++-
2 files changed, 144 insertions(+), 1 deletion(-)
diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
index 45fb6093dec2..7b0a0a8cdb6b 100644
--- a/Documentation/hwmon/max31785
+++ b/Documentation/hwmon/max31785
@@ -32,6 +32,7 @@ Sysfs attributes
fan[1-4]_alarm Fan alarm.
fan[1-4]_fault Fan fault.
fan[1-4]_input Fan RPM.
+fan[1-4]_target Fan input target
in[1-6]_crit Critical maximum output voltage
in[1-6]_crit_alarm Output voltage critical high alarm
@@ -44,6 +45,12 @@ in[1-6]_max_alarm Output voltage high alarm
in[1-6]_min Minimum output voltage
in[1-6]_min_alarm Output voltage low alarm
+pwm[1-4] Fan target duty cycle (0..255)
+pwm[1-4]_enable 0: Full-speed
+ 1: Manual PWM control
+ 2: Automatic PWM (tach-feedback RPM fan-control)
+ 3: Automatic closed-loop (temp-feedback fan-control)
+
temp[1-11]_crit Critical high temperature
temp[1-11]_crit_alarm Chip temperature critical high alarm
temp[1-11]_input Measured temperature
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 9313849d5160..8706a696c89a 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -20,8 +20,136 @@ enum max31785_regs {
#define MAX31785_NR_PAGES 23
+static int max31785_get_pwm(struct i2c_client *client, int page)
+{
+ int rv;
+
+ rv = pmbus_get_fan_rate_device(client, page, 0, percent);
+ if (rv < 0)
+ return rv;
+ else if (rv >= 0x8000)
+ return 0;
+ else if (rv >= 0x2711)
+ return 0x2710;
+
+ return rv;
+}
+
+static int max31785_get_pwm_mode(struct i2c_client *client, int page)
+{
+ int config;
+ int command;
+
+ config = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);
+ if (config < 0)
+ return config;
+
+ command = pmbus_read_word_data(client, page, PMBUS_FAN_COMMAND_1);
+ if (command < 0)
+ return command;
+
+ if (config & PB_FAN_1_RPM)
+ return (command >= 0x8000) ? 3 : 2;
+
+ if (command >= 0x8000)
+ return 3;
+ else if (command >= 0x2711)
+ return 0;
+
+ return 1;
+}
+
+static int max31785_read_word_data(struct i2c_client *client, int page,
+ int reg)
+{
+ int rv;
+
+ switch (reg) {
+ case PMBUS_VIRT_PWM_1:
+ rv = max31785_get_pwm(client, page);
+ break;
+ case PMBUS_VIRT_PWM_ENABLE_1:
+ rv = max31785_get_pwm_mode(client, page);
+ break;
+ default:
+ rv = -ENODATA;
+ break;
+ }
+
+ return rv;
+}
+
+static inline u32 max31785_scale_pwm(u32 sensor_val)
+{
+ /*
+ * The datasheet describes the accepted value range for manual PWM as
+ * [0, 0x2710], while the hwmon pwmX sysfs interface accepts values in
+ * [0, 255]. The MAX31785 uses DIRECT mode to scale the FAN_COMMAND
+ * registers and in PWM mode the coefficients are m=1, b=0, R=2. The
+ * important observation here is that 0x2710 == 10000 == 100 * 100.
+ *
+ * R=2 (== 10^2 == 100) accounts for scaling the value provided at the
+ * sysfs interface into the required hardware resolution, but it does
+ * not yet yield a value that we can write to the device (this initial
+ * scaling is handled by pmbus_data2reg()). Multiplying by 100 below
+ * translates the parameter value into the percentage units required by
+ * PMBus, and then we scale back by 255 as required by the hwmon pwmX
+ * interface to yield the percentage value at the appropriate
+ * resolution for hardware.
+ */
+ return (sensor_val * 100) / 255;
+}
+
+static int max31785_pwm_enable(struct i2c_client *client, int page,
+ u16 word)
+{
+ int config = 0;
+ int rate;
+
+ switch (word) {
+ case 0:
+ rate = 0x7fff;
+ break;
+ case 1:
+ rate = pmbus_get_fan_rate_cached(client, page, 0, percent);
+ if (rate < 0)
+ return rate;
+ rate = max31785_scale_pwm(rate);
+ break;
+ case 2:
+ config = PB_FAN_1_RPM;
+ rate = pmbus_get_fan_rate_cached(client, page, 0, rpm);
+ if (rate < 0)
+ return rate;
+ break;
+ case 3:
+ rate = 0xffff;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return pmbus_update_fan(client, page, 0, config, PB_FAN_1_RPM, rate);
+}
+
+static int max31785_write_word_data(struct i2c_client *client, int page,
+ int reg, u16 word)
+{
+ switch (reg) {
+ case PMBUS_VIRT_PWM_1:
+ return pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,
+ max31785_scale_pwm(word));
+ case PMBUS_VIRT_PWM_ENABLE_1:
+ return max31785_pwm_enable(client, page, word);
+ default:
+ break;
+ }
+
+ return -ENODATA;
+}
+
#define MAX31785_FAN_FUNCS \
- (PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12)
+ (PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_PWM12)
#define MAX31785_TEMP_FUNCS \
(PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
@@ -32,11 +160,19 @@ enum max31785_regs {
static const struct pmbus_driver_info max31785_info = {
.pages = MAX31785_NR_PAGES,
+ .write_word_data = max31785_write_word_data,
+ .read_word_data = max31785_read_word_data,
+
/* RPM */
.format[PSC_FAN] = direct,
.m[PSC_FAN] = 1,
.b[PSC_FAN] = 0,
.R[PSC_FAN] = 0,
+ /* PWM */
+ .format[PSC_PWM] = direct,
+ .m[PSC_PWM] = 1,
+ .b[PSC_PWM] = 0,
+ .R[PSC_PWM] = 2,
.func[0] = MAX31785_FAN_FUNCS,
.func[1] = MAX31785_FAN_FUNCS,
.func[2] = MAX31785_FAN_FUNCS,
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH v6 3/4] pmbus (core): Add virtual page config bit
From: Andrew Jeffery @ 2017-11-20 4:42 UTC (permalink / raw)
To: linux-hwmon-u79uwXL29TY76Z2rM5mHXA
Cc: Andrew Jeffery, linux-0h96xk9xTtrk1uMJSBkQmQ,
jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, joel-U3u1mxZcP9KHXe+LvDLADg,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <cover.dfb0a63462b38b9384b1d077fd62d17a11a7a0dd.1511152748.git-series.andrew-zrmu5oMJ5Fs@public.gmane.org>
Some circumstances call for virtual pages, to expose multiple values
packed into an extended PMBus register in a manner non-compliant with
the PMBus standard. An example of this is the Maxim MAX31785 controller, which
extends the READ_FAN_SPEED_1 PMBus register from two to four bytes to support
tach readings for both rotors of a dual rotor fan. This extended register
contains two word-sized values, one reporting the rate of the fastest rotor,
the other the rate of the slowest. The concept of virtual pages aids this
situation by mapping the page number onto the value to be selected from the
vectored result.
We should not try to set virtual pages on the device as such a page explicitly
doesn't exist; add a flag so we can avoid doing so.
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
drivers/hwmon/pmbus/pmbus.h | 2 ++
drivers/hwmon/pmbus/pmbus_core.c | 27 ++++++++++++++++++---------
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index b54d7604d3ef..d39d506aa63e 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -372,6 +372,8 @@ enum pmbus_sensor_classes {
#define PMBUS_HAVE_PWM12 BIT(20)
#define PMBUS_HAVE_PWM34 BIT(21)
+#define PMBUS_PAGE_VIRTUAL BIT(31)
+
enum pmbus_data_format { linear = 0, direct, vid };
enum vrm_version { vr11 = 0, vr12, vr13 };
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index edc25efe7552..e215bbd403a5 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -161,18 +161,27 @@ EXPORT_SYMBOL_GPL(pmbus_clear_cache);
int pmbus_set_page(struct i2c_client *client, int page)
{
struct pmbus_data *data = i2c_get_clientdata(client);
- int rv = 0;
- int newpage;
+ int rv;
+
+ if (page < 0 || page == data->currpage)
+ return 0;
- if (page >= 0 && page != data->currpage) {
+ if (!(data->info->func[page] & PMBUS_PAGE_VIRTUAL)) {
rv = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
- newpage = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
- if (newpage != page)
- rv = -EIO;
- else
- data->currpage = page;
+ if (rv < 0)
+ return rv;
+
+ rv = i2c_smbus_read_byte_data(client, PMBUS_PAGE);
+ if (rv < 0)
+ return rv;
+
+ if (rv != page)
+ return -EIO;
}
- return rv;
+
+ data->currpage = page;
+
+ return 0;
}
EXPORT_SYMBOL_GPL(pmbus_set_page);
--
git-series 0.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v6 4/4] pmbus (max31785): Add dual tachometer support
From: Andrew Jeffery @ 2017-11-20 4:42 UTC (permalink / raw)
To: linux-hwmon
Cc: Andrew Jeffery, linux, jdelvare, corbet, devicetree, linux-kernel,
linux-doc, joel, openbmc
In-Reply-To: <cover.dfb0a63462b38b9384b1d077fd62d17a11a7a0dd.1511152748.git-series.andrew@aj.id.au>
The dual tachometer feature is implemented in hardware with a TACHSEL
input to indicate the rotor under measurement, and exposed on the device
by extending the READ_FAN_SPEED_1 word with two extra bytes*. The need
to read the non-standard four-byte response leads to a cut-down
implementation of i2c_smbus_xfer_emulated() included in the driver.
Further, to expose the second rotor tachometer value to userspace the
values are exposed through virtual pages. We re-route accesses to
FAN_CONFIG_1_2 and READ_FAN_SPEED_1 on pages 23-28 (not defined by the
hardware) to the same registers on pages 0-5, and with the latter command we
extract the value from the second word of the four-byte response.
* The documentation recommends the slower rotor be associated with
TACHSEL=0, which corresponds to the first word of the response. The TACHSEL=0
measurement is used by the controller's closed-loop fan management to judge
target fan rate.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
Documentation/hwmon/max31785 | 8 +-
drivers/hwmon/pmbus/max31785.c | 147 ++++++++++++++++++++++++++++++++++-
2 files changed, 152 insertions(+), 3 deletions(-)
diff --git a/Documentation/hwmon/max31785 b/Documentation/hwmon/max31785
index 7b0a0a8cdb6b..270c5f865261 100644
--- a/Documentation/hwmon/max31785
+++ b/Documentation/hwmon/max31785
@@ -17,8 +17,9 @@ management with temperature and remote voltage sensing. Various fan control
features are provided, including PWM frequency control, temperature hysteresis,
dual tachometer measurements, and fan health monitoring.
-For dual rotor fan configuration, the MAX31785 exposes the slowest rotor of the
-two in the fan[1-4]_input attributes.
+For dual-rotor configurations the MAX31785A exposes the second rotor tachometer
+readings in attributes fan[5-8]_input. By contrast the MAX31785 only exposes
+the slowest rotor measurement, and does so in the fan[1-4]_input attributes.
Usage Notes
-----------
@@ -31,7 +32,8 @@ Sysfs attributes
fan[1-4]_alarm Fan alarm.
fan[1-4]_fault Fan fault.
-fan[1-4]_input Fan RPM.
+fan[1-8]_input Fan RPM. On the MAX31785A, inputs 5-8 correspond to the
+ second rotor of fans 1-4
fan[1-4]_target Fan input target
in[1-6]_crit Critical maximum output voltage
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index 8706a696c89a..bffab449be39 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -16,9 +16,79 @@
enum max31785_regs {
MFR_REVISION = 0x9b,
+ MFR_FAN_CONFIG = 0xf1,
};
+#define MAX31785 0x3030
+#define MAX31785A 0x3040
+
+#define MFR_FAN_CONFIG_DUAL_TACH BIT(12)
+
#define MAX31785_NR_PAGES 23
+#define MAX31785_NR_FAN_PAGES 6
+
+static int max31785_read_byte_data(struct i2c_client *client, int page,
+ int reg)
+{
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ switch (reg) {
+ case PMBUS_VOUT_MODE:
+ return -ENOTSUPP;
+ case PMBUS_FAN_CONFIG_12:
+ return pmbus_read_byte_data(client, page - MAX31785_NR_PAGES,
+ reg);
+ }
+
+ return -ENODATA;
+}
+
+static int max31785_write_byte(struct i2c_client *client, int page, u8 value)
+{
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ return -ENOTSUPP;
+}
+
+static int max31785_read_long_data(struct i2c_client *client, int page,
+ int reg, u32 *data)
+{
+ unsigned char cmdbuf[1];
+ unsigned char rspbuf[4];
+ int rc;
+
+ struct i2c_msg msg[2] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = sizeof(cmdbuf),
+ .buf = cmdbuf,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = sizeof(rspbuf),
+ .buf = rspbuf,
+ },
+ };
+
+ cmdbuf[0] = reg;
+
+ rc = pmbus_set_page(client, page);
+ if (rc < 0)
+ return rc;
+
+ rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (rc < 0)
+ return rc;
+
+ *data = (rspbuf[0] << (0 * 8)) | (rspbuf[1] << (1 * 8)) |
+ (rspbuf[2] << (2 * 8)) | (rspbuf[3] << (3 * 8));
+
+ return rc;
+}
static int max31785_get_pwm(struct i2c_client *client, int page)
{
@@ -62,9 +132,30 @@ static int max31785_get_pwm_mode(struct i2c_client *client, int page)
static int max31785_read_word_data(struct i2c_client *client, int page,
int reg)
{
+ u32 val;
int rv;
switch (reg) {
+ case PMBUS_READ_FAN_SPEED_1:
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ rv = max31785_read_long_data(client, page - MAX31785_NR_PAGES,
+ reg, &val);
+ if (rv < 0)
+ return rv;
+
+ rv = (val >> 16) & 0xffff;
+ break;
+ case PMBUS_FAN_COMMAND_1:
+ /*
+ * PMBUS_FAN_COMMAND_x is probed to judge whether or not to
+ * expose fan control registers.
+ *
+ * Don't expose fan_target attribute for virtual pages.
+ */
+ rv = (page >= MAX31785_NR_PAGES) ? -ENOTSUPP : -ENODATA;
+ break;
case PMBUS_VIRT_PWM_1:
rv = max31785_get_pwm(client, page);
break;
@@ -157,11 +248,15 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
#define MAX31785_VOUT_FUNCS \
(PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT)
+#define MAX37185_NUM_FAN_PAGES 6
+
static const struct pmbus_driver_info max31785_info = {
.pages = MAX31785_NR_PAGES,
.write_word_data = max31785_write_word_data,
+ .read_byte_data = max31785_read_byte_data,
.read_word_data = max31785_read_word_data,
+ .write_byte = max31785_write_byte,
/* RPM */
.format[PSC_FAN] = direct,
@@ -208,13 +303,46 @@ static const struct pmbus_driver_info max31785_info = {
.func[22] = MAX31785_VOUT_FUNCS,
};
+static int max31785_configure_dual_tach(struct i2c_client *client,
+ struct pmbus_driver_info *info)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < MAX31785_NR_FAN_PAGES; i++) {
+ ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_word_data(client, MFR_FAN_CONFIG);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MFR_FAN_CONFIG_DUAL_TACH) {
+ int virtual = MAX31785_NR_PAGES + i;
+
+ info->pages = virtual + 1;
+ info->func[virtual] |= PMBUS_HAVE_FAN12;
+ info->func[virtual] |= PMBUS_PAGE_VIRTUAL;
+ }
+ }
+
+ return 0;
+}
+
static int max31785_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct device *dev = &client->dev;
struct pmbus_driver_info *info;
+ bool dual_tach = false;
s64 ret;
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_BYTE_DATA |
+ I2C_FUNC_SMBUS_WORD_DATA))
+ return -ENODEV;
+
info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
if (!info)
return -ENOMEM;
@@ -225,6 +353,25 @@ static int max31785_probe(struct i2c_client *client,
if (ret < 0)
return ret;
+ ret = i2c_smbus_read_word_data(client, MFR_REVISION);
+ if (ret < 0)
+ return ret;
+
+ if (ret == MAX31785A) {
+ dual_tach = true;
+ } else if (ret == MAX31785) {
+ if (!strcmp("max31785a", id->name))
+ dev_warn(dev, "Expected max3175a, found max31785: cannot provide secondary tachometer readings\n");
+ } else {
+ return -ENODEV;
+ }
+
+ if (dual_tach) {
+ ret = max31785_configure_dual_tach(client, info);
+ if (ret < 0)
+ return ret;
+ }
+
return pmbus_do_probe(client, id, info);
}
--
git-series 0.9.1
^ permalink raw reply related
* Re: [PATCH v2] iio: adc: aspeed: Deassert reset in probe
From: Joel Stanley @ 2017-11-20 4:52 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Jonathan Cameron, Philipp Zabel, Rick Altherr, Rob Herring,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio-u79uwXL29TY76Z2rM5mHXA, devicetree, LKML
In-Reply-To: <20171119160318.41d7e85a@archlinux>
On Mon, Nov 20, 2017 at 2:33 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, 2 Nov 2017 14:49:32 +0000
> Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
>> IIO is closed for this cycle anyway now.
>> Otherwise, series looks good.
>>
>> Will pick up when back with my main PC as traveling for this week and
>> next.
>
> Forgot to ask, do you want me to pick this up as a fix?
> Also does it make sense to tag it for stable?
>
> If not I can pick it up for the coming cycle. Given the code changes
> are small and well isolated I'm happy to do any of the 3 options,
> it really depends on whether the rest of the platform works well enough
> to be worth rushing these through?
Without the clock driver upstream there's no rush to merge this. I'm
still waiting on review from the clock guys.
Please queue them for 4.16. Thanks!
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH v4 4/5] DTS: gta04: add serdev node for w2sg00x4
From: H. Nikolaus Schaller @ 2017-11-20 5:29 UTC (permalink / raw)
To: Tony Lindgren
Cc: Rob Herring, Mark Rutland, Benoît Cousson, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, Kevin Hilman,
Andreas Färber, Thierry Reding, Jonathan Cameron,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
letux-kernel-S0jZdbWzriLCfDggNXIi3w,
kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171117170857.GO28152-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Hi Tony,
> Am 17.11.2017 um 18:08 schrieb Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
>
> * H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> [171115 21:41]:
>> GTA04 has a W2SG0004/84 connected to UART2 of the OMAP3
>> processor. A GPIO can pulse the on/off toggle switch.
>>
>> The VSIM regulator is used to power on/off the LNA of an external
>> active GPS antenna so that a driver can turn the LNA off if GPS is
>> not needed to save battery energy.
>>
>> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/omap3-gta04.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
>> index 4504908c23fe..e967d738aaaf 100644
>> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
>> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
>> @@ -477,6 +477,12 @@
>> &uart2 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&uart2_pins>;
>> +
>> + gps: gps {
>> + compatible = "wi2wi,w2sg0004";
>> + lna-supply = <&vsim>; /* LNA regulator */
>> + enable-gpios = <&gpio5 17 GPIO_ACTIVE_HIGH>; /* GPIO_145: trigger for turning on/off w2sg0004 */
>> + };
>> };
>
> How about move the comment above the node to avoid long lines?
> Or just change it to gpio_145 for the comment as the property
> is already enable-gpios?
>
> Regards,
>
> Tony
Done so that it will be included in next version.
BR and thanks,
Nikolaus
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/4] omapdrm: fix compatible string for td028ttec1
From: H. Nikolaus Schaller @ 2017-11-20 5:36 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Mark Rutland, DTML, linux-fbdev,
Discussions about the Letux Kernel, Bartlomiej Zolnierkiewicz,
David Airlie, Tomi Valkeinen, Russell King, dri-devel,
Linux Kernel Mailing List, Julia Lawall, Tony Lindgren,
Rob Herring, Thierry Reding, Laurent Pinchart,
Benoît Cousson, kernel, Sean Paul, linux-omap, Linux ARM
In-Reply-To: <D565D101-4EAF-4B3C-907F-A826EF53B5AF@goldelico.com>
Hi Andrew,
> Am 16.11.2017 um 19:56 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>
> Hi Andrew,
>
>> Am 16.11.2017 um 19:32 schrieb Andrew F. Davis <afd@ti.com>:
>>
>> On 11/16/2017 12:18 PM, H. Nikolaus Schaller wrote:
>>>
>>>> Am 16.11.2017 um 18:08 schrieb Andrew F. Davis <afd@ti.com>:
>>>>
>>>> On 11/16/2017 10:10 AM, H. Nikolaus Schaller wrote:
>>>>> Hi Andrew,
>>>>>
>>>>>> Am 16.11.2017 um 16:53 schrieb Andrew F. Davis <afd@ti.com>:
>>>>>>
>>>>>> On 11/16/2017 07:43 AM, H. Nikolaus Schaller wrote:
>>>>>>>
>>>>>>>> Am 16.11.2017 um 13:32 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>>>>>>>>
>>>>>>>> On 16/11/17 10:50, H. Nikolaus Schaller wrote:
>>>>>>>>> The vendor name was "toppoly" but other panels and the vendor list
>>>>>>>>> have defined it as "tpo". So let's fix it in driver and bindings.
>>>>>>>>>
>>>>>>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>>>>>>> ---
>>>>>>>>
>>>>>>>>
>>>>>>>>> -MODULE_ALIAS("spi:toppoly,td028ttec1");
>>>>>>>>> +MODULE_ALIAS("spi:tpo,td028ttec1");
>>>>>>>>
>>>>>>>> Doesn't this mean that the module won't load if you have old bindings?
>>>>>>>
>>>>>>> Hm.
>>>>>>>
>>>>>>> Well, I think it can load but doesn't automatically from DT strings which might
>>>>>>> be unexpected.
>>>>>>>
>>>>>>>> Can't we have two module aliases?
>>>>>>>
>>>>>>> I think we can. Just a random example:
>>>>>>> https://elixir.free-electrons.com/linux/latest/source/drivers/w1/slaves/w1_therm.c#L754
>>>>>>>
>>>>>>> So we should keep both.
>>>>>>
>>>>>> Even better would be to drop both MODULE_ALIAS and let the
>>>>>> MODULE_DEVICE_TABLE macro define them for your from the SPI id table.
>>>>>
>>>>> Why would that be better?
>>>>>
>>>>
>>>> MODULE_ALIAS is ugly, you already have a table (usually) of device names
>>>> that are supported by the driver, the module aliases should be generated
>>>> from that table. This also keeps supported device list in one place.
>>>>
>>>>> As far as I see it will need more code and changes than adding one line of
>>>>> MODULE_ALIAS.
>>>>>
>>>>>> Although it doesn't look like this driver has an SPI id table, you
>>>>>> should probably add one, I be interested to see if this module is always
>>>>>> being matched through the "spi" or the "of" alias..
>>>>>
>>>>> Could you please propose how that code should look like, so that I can test?
>>>>>
>>>>
>>>> Sure,
>>>>
>>>> start with
>>>> $ udevadm monitor
>>>> and see what string the kernel is looking for when trying to find a
>>>> module for this device.
>>>
>>> Well, the module is loaded automatically from DT at boot time well before
>>> I can start udevadm. So that is the most tricky part to setup the system
>>> to suppress this...
>>>
>>>>
>>>> If it is only ever looking for "of:toppoly,td028ttec1", then you can
>>>> drop the MODULE_ALIAS and be done as it was never getting used anyway.
>>>
>>> Since it is an SPI client, I am sure it looks for "spi:something.
>>>
>>>>
>>>> What I expect though is "spi:toppoly,td028ttec1", in which case you
>>>> should add
>>>>
>>>> static const struct spi_device_id td028ttec1_ids[] = {
>>>> { "toppoly,td028ttec1", 0 },
>>>> { "tpo,td028ttec1", 0},
>>>> { /* sentinel */ }
>>>> };
>>>> MODULE_DEVICE_TABLE(spi, td028ttec1_ids);
>>>
>>> We already have a static const struct of_device_id td028ttec1_of_match[]
>>> table with the same information.
>>>
>>> So we still have two places to keep in sync.
>>>
>>> Or can we remove the td028ttec1_of_match[]? AFAIK not.
>>>
>>>>
>>>> link to it in the td028ttec1_spi_driver struct:
>>>> .id_table = td028ttec1_ids,
>>>>
>>>> Then test again to see that the module still loads with the new and old
>>>> DT string.
>>>
>>> In total I am not really convinced that adding 7 lines of code is better
>>> than one (the "tpo," alias) that is tested and works...
>>>
>>> And it looks like a lot of unplanned code testing for me which takes more
>>> than 5 minutes :)
>>>
>>> So I'd prefer to leave that exercise of fixing the MODULE_ALIAS/DEVICE_TABLE
>>> to someone else...
>>>
>>
>> That's fine, someday I'll probably get some script to do this for all
>> the drivers that still have MODULE_ALIAS and an existing table.
>
> That would be cool!
>
> On a second thought, I think there is a quick experiment for this driver
> not needing to monitor events.
>
> 1st attempt: remove ALIASES => if it still loads it would be fine
> 2nd attempt: add your id table => if it loads again, it is fine
> if not, let's keep ALIASES.
>
> Maybe I can try tomorrow.
I found time to give it a try and indeed:
1. with module_aliases
root@letux:~# lsmod | fgrep td028
panel_tpo_td028ttec1 16384 1
omapdss_base 16384 5 connector_analog_tv,encoder_opa362,panel_tpo_td028ttec1,omapdrm,omapdss
root@letux:~# modprobe -c | fgrep td028
alias of:N*T*Comapdss,toppoly,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,toppoly,td028ttec1C* panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1C* panel_tpo_td028ttec1
alias spi:toppoly,td028ttec1 panel_tpo_td028ttec1
alias spi:tpo,td028ttec1 panel_tpo_td028ttec1
root@letux:~#
=> two MODULE_ALIASEs are possible.
2. without module_aliases
No display and:
root@letux:~# lsmod | fgrep td028
root@letux:~# modprobe -c | fgrep td028
alias of:N*T*Comapdss,toppoly,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,toppoly,td028ttec1C* panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1C* panel_tpo_td028ttec1
root@letux:~#
=> spi: entries are needed.
3. with MODULE_DEVICE_TABLE
Display is back and:
root@letux:~# lsmod | fgrep td028
panel_tpo_td028ttec1 16384 1
omapdss_base 16384 5 connector_analog_tv,encoder_opa362,panel_tpo_td028ttec1,omapdrm,omapdss
root@letux:~# modprobe -c | fgrep td028
alias of:N*T*Comapdss,toppoly,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,toppoly,td028ttec1C* panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1 panel_tpo_td028ttec1
alias of:N*T*Comapdss,tpo,td028ttec1C* panel_tpo_td028ttec1
alias spi:toppoly,td028ttec1 panel_tpo_td028ttec1
alias spi:tpo,td028ttec1 panel_tpo_td028ttec1
root@letux:~#
=> MODULE_DEVICE_TABLE works equally well.
I have prepared a separate patch so that a first one adds
another MODULE_ALIAS and the new one replaces both MODULE_ALIASes
with MODULE_DEVICE_TABLE.
I will submit a [PATCH v3] asap.
BR and thanks,
Nikolaus
^ permalink raw reply
* [PATCH] DT: i2c: W83773G is a trivial device
From: Lei YU @ 2017-11-20 5:53 UTC (permalink / raw)
To: Rob Herring; +Cc: Lei YU, devicetree-u79uwXL29TY76Z2rM5mHXA
Signed-off-by: Lei YU <mine260309-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/devicetree/bindings/trivial-devices.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
index af284fb..3c393e3 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -151,6 +151,7 @@ national,lm85 Temperature sensor with integrated fan control
national,lm92 ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator with Two-Wire Interface
nuvoton,npct501 i2c trusted platform module (TPM)
nuvoton,npct601 i2c trusted platform module (TPM2)
+nuvoton,w83773g Nuvoton Temperature Sensor
nxp,pca9556 Octal SMBus and I2C registered interface
nxp,pca9557 8-bit I2C-bus and SMBus I/O port with reset
nxp,pcf2127 Real-time clock
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH V5 00/12] add clock driver for Spreadtrum platforms
From: Chunyan Zhang @ 2017-11-20 6:01 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, Rob Herring, Mark Rutland
Cc: Catalin Marinas, Will Deacon, linux-clk, linux-kernel, devicetree,
linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
Ben Li, Orson Zhai, Chunyan Zhang
This series adds Spreadtrum clock support together with its binding
documentation and devicetree data.
Any comments would be greatly appreciated.
This patchset also added a few common clock mactos into drivers/clk/clk_common.h,
which are generally useful for all vendors' clock driver, sunxi-ng, zte, sprd
(added in this patchse) use them (or part of them) at present, once this patchset
is merged, I can help to remove the duplicated code which is under the vendors'
respective directories.
Thanks,
Chunyan
Changes from V4: (https://www.spinics.net/lists/arm-kernel/msg616566.html)
* Added Rob's Acked-by;
* Removed spin lock from Spreadtrum's gate, mux, div drivers, since we have
switched to use regmap.
Changes from V3: (https://lkml.org/lkml/2017/11/2/61)
* Addressed comments from Julien Thierry:
- Clean the if branch of sprd_mux_helper_get_parent()
- Have the Gate clock macros and ops for both mode (i.e. sc_gate and gate) separate;
- Have the Mux clock macros with/without table separate, and same changes
for the composite clock.
* Switched the function name from _endisable to _toggle;
* Fixed Kbuild test error:
- Added exporting sprd_clk_regmap_init() which would be used in other module(s);
* Change the function sprd_clk_set_regmap() to the static one, and removed the
declear from the include file;
* Addressed comments from Rob:
- Separate the dt-binding include file from the driver patch;
- Documented more for the property "clocks"
* Changed the syscon device names;
* Changed the name of 'sprd_mux_internal' to 'sprd_mux_ssel'
Changes from V2: (http://lkml.iu.edu/hypermail/linux/kernel/1707.1/01504.html)
* Switch to use regmap to access registers;
* Splited all clocks into 16 separated nodes, for each belongs to a single address area;
* Rearranged the order of clock declaration in sc9860-clk.c, sorted them upon the address area;
* Added syscon device tree nodes which will be quoted by the node of clocks which are in
the same address area with the syscon device;
* Revised the binding documentation according to the dt modification.
Changes from V1: (https://lkml.org/lkml/2017/6/17/356)
* Address Stephen's comments:
- Switch to use platform device driver instead of the DT probing mechanism.
- Move the common clock macro out from vendor directory, but need to remove those
overlap code from other vendors (such as sunxi-ng) once this get merged.
- Add support to be built as a module.
- Add 'sprd_' prefix for all spin locks used in these drivers.
- Mark input parameter of sprd_x with const.
- Remove unreasonable dependencies to CONFIG_64BIT.
- Add readl() after writing the same register.
- Remove CLK_IS_BASIC which is no longer used.
- Remove unnecessery CLK_IGNORE_UNUSED when defining a clock.
- Change to expose all clock index.
- Use clk_ instead of ccu.
- Add Kconfig for sprd clocks.
- Move the fixed clocks out from the soc node.
- Switch to use 64-bit math in pll driver instead of 32-bit math.
* Revise binding documentation according to dt modification.
* Rename sc9860.c to sc9860-clk.c
Chunyan Zhang (12):
drivers: move clock common macros out from vendor directories
clk: sprd: Add common infrastructure
clk: sprd: add gate clock support
clk: sprd: add mux clock support
clk: sprd: add divider clock support
clk: sprd: add composite clock support
clk: sprd: add adjustable pll support
dt-bindings: Add Spreadtrum clock binding documentation
clk: sprd: Add dt-bindings include file for SC9860
clk: sprd: add clocks support for SC9860
arm64: dts: add syscon for whale2 platform
arm64: dts: add clocks for SC9860
Documentation/devicetree/bindings/clock/sprd.txt | 63 +
arch/arm64/boot/dts/sprd/sc9860.dtsi | 115 ++
arch/arm64/boot/dts/sprd/whale2.dtsi | 48 +-
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/clk_common.h | 60 +
drivers/clk/sprd/Kconfig | 14 +
drivers/clk/sprd/Makefile | 11 +
drivers/clk/sprd/common.c | 113 ++
drivers/clk/sprd/common.h | 53 +
drivers/clk/sprd/composite.c | 62 +
drivers/clk/sprd/composite.h | 53 +
drivers/clk/sprd/div.c | 92 +
drivers/clk/sprd/div.h | 77 +
drivers/clk/sprd/gate.c | 113 ++
drivers/clk/sprd/gate.h | 61 +
drivers/clk/sprd/mux.c | 78 +
drivers/clk/sprd/mux.h | 76 +
drivers/clk/sprd/pll.c | 268 +++
drivers/clk/sprd/pll.h | 110 ++
drivers/clk/sprd/sc9860-clk.c | 1987 ++++++++++++++++++++++
include/dt-bindings/clock/sprd,sc9860-clk.h | 408 +++++
22 files changed, 3862 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/sprd.txt
create mode 100644 drivers/clk/clk_common.h
create mode 100644 drivers/clk/sprd/Kconfig
create mode 100644 drivers/clk/sprd/Makefile
create mode 100644 drivers/clk/sprd/common.c
create mode 100644 drivers/clk/sprd/common.h
create mode 100644 drivers/clk/sprd/composite.c
create mode 100644 drivers/clk/sprd/composite.h
create mode 100644 drivers/clk/sprd/div.c
create mode 100644 drivers/clk/sprd/div.h
create mode 100644 drivers/clk/sprd/gate.c
create mode 100644 drivers/clk/sprd/gate.h
create mode 100644 drivers/clk/sprd/mux.c
create mode 100644 drivers/clk/sprd/mux.h
create mode 100644 drivers/clk/sprd/pll.c
create mode 100644 drivers/clk/sprd/pll.h
create mode 100644 drivers/clk/sprd/sc9860-clk.c
create mode 100644 include/dt-bindings/clock/sprd,sc9860-clk.h
--
2.7.4
^ permalink raw reply
* [PATCH V5 01/12] drivers: move clock common macros out from vendor directories
From: Chunyan Zhang @ 2017-11-20 6:01 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, Rob Herring, Mark Rutland
Cc: Catalin Marinas, Will Deacon, linux-clk, linux-kernel, devicetree,
linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171120060137.27380-1-chunyan.zhang@spreadtrum.com>
These macros are used by more than one SoC vendor platforms, avoid to
have many copies of these code, this patch moves them to the common
clock directory which every clock drivers can access to.
Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
drivers/clk/clk_common.h | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 drivers/clk/clk_common.h
diff --git a/drivers/clk/clk_common.h b/drivers/clk/clk_common.h
new file mode 100644
index 0000000..21e93d2
--- /dev/null
+++ b/drivers/clk/clk_common.h
@@ -0,0 +1,60 @@
+/*
+ * drivers/clk/clk_common.h
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _CLK_COMMON_H_
+#define _CLK_COMMON_H_
+
+#include <linux/clk-provider.h>
+
+#define CLK_HW_INIT(_name, _parent, _ops, _flags) \
+ (&(struct clk_init_data) { \
+ .flags = _flags, \
+ .name = _name, \
+ .parent_names = (const char *[]) { _parent }, \
+ .num_parents = 1, \
+ .ops = _ops, \
+ })
+
+#define CLK_HW_INIT_PARENTS(_name, _parents, _ops, _flags) \
+ (&(struct clk_init_data) { \
+ .flags = _flags, \
+ .name = _name, \
+ .parent_names = _parents, \
+ .num_parents = ARRAY_SIZE(_parents), \
+ .ops = _ops, \
+ })
+
+#define CLK_HW_INIT_NO_PARENT(_name, _ops, _flags) \
+ (&(struct clk_init_data) { \
+ .flags = _flags, \
+ .name = _name, \
+ .parent_names = NULL, \
+ .num_parents = 0, \
+ .ops = _ops, \
+ })
+
+#define CLK_FIXED_FACTOR(_struct, _name, _parent, \
+ _div, _mult, _flags) \
+ struct clk_fixed_factor _struct = { \
+ .div = _div, \
+ .mult = _mult, \
+ .hw.init = CLK_HW_INIT(_name, \
+ _parent, \
+ &clk_fixed_factor_ops, \
+ _flags), \
+ }
+
+#define CLK_FIXED_RATE(_struct, _name, _flags, \
+ _fixed_rate, _fixed_accuracy) \
+ struct clk_fixed_rate _struct = { \
+ .fixed_rate = _fixed_rate, \
+ .fixed_accuracy = _fixed_accuracy, \
+ .hw.init = CLK_HW_INIT_NO_PARENT(_name, \
+ &clk_fixed_rate_ops, \
+ _flags), \
+ }
+
+#endif /* _CLK_COMMON_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH V5 02/12] clk: sprd: Add common infrastructure
From: Chunyan Zhang @ 2017-11-20 6:01 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, Rob Herring, Mark Rutland
Cc: Catalin Marinas, Will Deacon, linux-clk, linux-kernel, devicetree,
linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171120060137.27380-1-chunyan.zhang@spreadtrum.com>
Added Spreadtrum's clock driver framework together with common
structures and interface functions.
Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/sprd/Kconfig | 4 ++
drivers/clk/sprd/Makefile | 3 ++
drivers/clk/sprd/common.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/sprd/common.h | 53 ++++++++++++++++++++++
6 files changed, 175 insertions(+)
create mode 100644 drivers/clk/sprd/Kconfig
create mode 100644 drivers/clk/sprd/Makefile
create mode 100644 drivers/clk/sprd/common.c
create mode 100644 drivers/clk/sprd/common.h
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1c4e1aa..ce1a32be 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -236,6 +236,7 @@ source "drivers/clk/mvebu/Kconfig"
source "drivers/clk/qcom/Kconfig"
source "drivers/clk/renesas/Kconfig"
source "drivers/clk/samsung/Kconfig"
+source "drivers/clk/sprd/Kconfig"
source "drivers/clk/sunxi-ng/Kconfig"
source "drivers/clk/tegra/Kconfig"
source "drivers/clk/ti/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index c99f363..fa33891 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_COMMON_CLK_SAMSUNG) += samsung/
obj-$(CONFIG_ARCH_SIRF) += sirf/
obj-$(CONFIG_ARCH_SOCFPGA) += socfpga/
obj-$(CONFIG_PLAT_SPEAR) += spear/
+obj-$(CONFIG_ARCH_SPRD) += sprd/
obj-$(CONFIG_ARCH_STI) += st/
obj-$(CONFIG_ARCH_SUNXI) += sunxi/
obj-$(CONFIG_ARCH_SUNXI) += sunxi-ng/
diff --git a/drivers/clk/sprd/Kconfig b/drivers/clk/sprd/Kconfig
new file mode 100644
index 0000000..67a3287
--- /dev/null
+++ b/drivers/clk/sprd/Kconfig
@@ -0,0 +1,4 @@
+config SPRD_COMMON_CLK
+ tristate "Clock support for Spreadtrum SoCs"
+ depends on ARCH_SPRD || COMPILE_TEST
+ default ARCH_SPRD
diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
new file mode 100644
index 0000000..74f4b80
--- /dev/null
+++ b/drivers/clk/sprd/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_SPRD_COMMON_CLK) += clk-sprd.o
+
+clk-sprd-y += common.o
diff --git a/drivers/clk/sprd/common.c b/drivers/clk/sprd/common.c
new file mode 100644
index 0000000..c003f09
--- /dev/null
+++ b/drivers/clk/sprd/common.c
@@ -0,0 +1,113 @@
+/*
+ * Spreadtrum clock infrastructure
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+
+#include "common.h"
+
+static const struct regmap_config sprdclk_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0xffff,
+ .fast_io = true,
+};
+
+static void sprd_clk_set_regmap(const struct sprd_clk_desc *desc,
+ struct regmap *regmap)
+{
+ int i;
+ struct sprd_clk_common *cclk;
+
+ for (i = 0; i < desc->num_clk_clks; i++) {
+ cclk = desc->clk_clks[i];
+ if (!cclk)
+ continue;
+
+ cclk->regmap = regmap;
+ }
+}
+
+int sprd_clk_regmap_init(struct platform_device *pdev,
+ const struct sprd_clk_desc *desc)
+{
+ void __iomem *base;
+ struct device_node *node = pdev->dev.of_node;
+ struct regmap *regmap = NULL;
+
+ if (of_find_property(node, "sprd,syscon", NULL)) {
+ regmap = syscon_regmap_lookup_by_phandle(node, "sprd,syscon");
+ if (IS_ERR(regmap)) {
+ pr_err("%s: failed to get syscon regmap\n", __func__);
+ return PTR_ERR(regmap);
+ }
+ } else {
+ base = of_iomap(node, 0);
+ regmap = devm_regmap_init_mmio(&pdev->dev, base,
+ &sprdclk_regmap_config);
+ if (IS_ERR(regmap)) {
+ pr_err("failed to init regmap.\n");
+ return PTR_ERR(regmap);
+ }
+ }
+
+ sprd_clk_set_regmap(desc, regmap);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sprd_clk_regmap_init);
+
+int sprd_clk_probe(struct device_node *node,
+ struct clk_hw_onecell_data *clkhw)
+{
+ int i, ret = 0;
+ struct clk_hw *hw;
+
+ for (i = 0; i < clkhw->num; i++) {
+
+ hw = clkhw->hws[i];
+
+ if (!hw)
+ continue;
+
+ ret = clk_hw_register(NULL, hw);
+ if (ret) {
+ pr_err("Couldn't register clock %d - %s\n",
+ i, hw->init->name);
+ goto err_clk_unreg;
+ }
+ }
+
+ ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
+ clkhw);
+ if (ret) {
+ pr_err("Failed to add clock provider.\n");
+ goto err_clk_unreg;
+ }
+
+ return 0;
+
+err_clk_unreg:
+ while (--i >= 0) {
+ hw = clkhw->hws[i];
+ if (!hw)
+ continue;
+
+ clk_hw_unregister(hw);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(sprd_clk_probe);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/sprd/common.h b/drivers/clk/sprd/common.h
new file mode 100644
index 0000000..a25634a
--- /dev/null
+++ b/drivers/clk/sprd/common.h
@@ -0,0 +1,53 @@
+/*
+ * Spreadtrum clock infrastructure
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _SPRD_CLK_COMMON_H_
+#define _SPRD_CLK_COMMON_H_
+
+#include <linux/clk-provider.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+
+#include "../clk_common.h"
+
+struct device_node;
+
+struct sprd_clk_common {
+ struct regmap *regmap;
+ u32 reg;
+ struct clk_hw hw;
+};
+
+struct sprd_clk_desc {
+ struct sprd_clk_common **clk_clks;
+ unsigned long num_clk_clks;
+ struct clk_hw_onecell_data *hw_clks;
+};
+
+#define sprd_regmap_read(map, reg, val) \
+({ \
+ (map) ? regmap_read((map), (reg), (val)) : (-EINVAL); \
+})
+
+#define sprd_regmap_write(map, reg, val) \
+({ \
+ (map) ? regmap_write((map), (reg), (val)) : (-EINVAL); \
+})
+
+static inline struct sprd_clk_common *
+ hw_to_sprd_clk_common(const struct clk_hw *hw)
+{
+ return container_of(hw, struct sprd_clk_common, hw);
+}
+int sprd_clk_regmap_init(struct platform_device *pdev,
+ const struct sprd_clk_desc *desc);
+int sprd_clk_probe(struct device_node *node,
+ struct clk_hw_onecell_data *clkhw);
+
+#endif /* _SPRD_CLK_COMMON_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH V5 03/12] clk: sprd: add gate clock support
From: Chunyan Zhang @ 2017-11-20 6:01 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, Rob Herring, Mark Rutland
Cc: Catalin Marinas, Will Deacon, linux-clk, linux-kernel, devicetree,
linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171120060137.27380-1-chunyan.zhang@spreadtrum.com>
Some clocks on the Spreadtrum's SoCs are just simple gates. Add
support for those clocks.
Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
drivers/clk/sprd/Makefile | 1 +
drivers/clk/sprd/gate.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/sprd/gate.h | 61 +++++++++++++++++++++++++
3 files changed, 175 insertions(+)
create mode 100644 drivers/clk/sprd/gate.c
create mode 100644 drivers/clk/sprd/gate.h
diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
index 74f4b80..8cd5592 100644
--- a/drivers/clk/sprd/Makefile
+++ b/drivers/clk/sprd/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_SPRD_COMMON_CLK) += clk-sprd.o
clk-sprd-y += common.o
+clk-sprd-y += gate.o
diff --git a/drivers/clk/sprd/gate.c b/drivers/clk/sprd/gate.c
new file mode 100644
index 0000000..5dcc6a8
--- /dev/null
+++ b/drivers/clk/sprd/gate.c
@@ -0,0 +1,113 @@
+/*
+ * Spreadtrum gate clock driver
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+
+#include "gate.h"
+
+static void clk_gate_toggle(const struct sprd_gate *sg, bool en)
+{
+ const struct sprd_clk_common *common = &sg->common;
+ unsigned int reg;
+ bool set = sg->flags & CLK_GATE_SET_TO_DISABLE ? true : false;
+
+ set ^= en;
+
+ sprd_regmap_read(common->regmap, common->reg, ®);
+
+ if (set)
+ reg |= sg->enable_mask;
+ else
+ reg &= ~sg->enable_mask;
+
+ sprd_regmap_write(common->regmap, common->reg, reg);
+}
+
+static void clk_sc_gate_toggle(const struct sprd_gate *sg, bool en)
+{
+ const struct sprd_clk_common *common = &sg->common;
+ bool set = sg->flags & CLK_GATE_SET_TO_DISABLE ? 1 : 0;
+ unsigned int offset;
+
+ set ^= en;
+
+ /*
+ * Each set/clear gate clock has three registers:
+ * common->reg - base register
+ * common->reg + offset - set register
+ * common->reg + 2 * offset - clear register
+ */
+ offset = set ? sg->sc_offset : sg->sc_offset * 2;
+
+ sprd_regmap_write(common->regmap, common->reg + offset,
+ sg->enable_mask);
+}
+
+static void sprd_gate_disable(struct clk_hw *hw)
+{
+ struct sprd_gate *sg = hw_to_sprd_gate(hw);
+
+ clk_gate_toggle(sg, false);
+}
+
+static int sprd_gate_enable(struct clk_hw *hw)
+{
+ struct sprd_gate *sg = hw_to_sprd_gate(hw);
+
+ clk_gate_toggle(sg, true);
+
+ return 0;
+}
+
+static void sprd_sc_gate_disable(struct clk_hw *hw)
+{
+ struct sprd_gate *sg = hw_to_sprd_gate(hw);
+
+ clk_sc_gate_toggle(sg, false);
+}
+
+static int sprd_sc_gate_enable(struct clk_hw *hw)
+{
+ struct sprd_gate *sg = hw_to_sprd_gate(hw);
+
+ clk_sc_gate_toggle(sg, true);
+
+ return 0;
+}
+static int sprd_gate_is_enabled(struct clk_hw *hw)
+{
+ struct sprd_gate *sg = hw_to_sprd_gate(hw);
+ struct sprd_clk_common *common = &sg->common;
+ unsigned int reg;
+
+ sprd_regmap_read(common->regmap, common->reg, ®);
+
+ if (sg->flags & CLK_GATE_SET_TO_DISABLE)
+ reg ^= sg->enable_mask;
+
+ reg &= sg->enable_mask;
+
+ return reg ? 1 : 0;
+}
+
+const struct clk_ops sprd_gate_ops = {
+ .disable = sprd_gate_disable,
+ .enable = sprd_gate_enable,
+ .is_enabled = sprd_gate_is_enabled,
+};
+EXPORT_SYMBOL_GPL(sprd_gate_ops);
+
+const struct clk_ops sprd_sc_gate_ops = {
+ .disable = sprd_sc_gate_disable,
+ .enable = sprd_sc_gate_enable,
+ .is_enabled = sprd_gate_is_enabled,
+};
+EXPORT_SYMBOL_GPL(sprd_sc_gate_ops);
+
diff --git a/drivers/clk/sprd/gate.h b/drivers/clk/sprd/gate.h
new file mode 100644
index 0000000..72e5b1a5
--- /dev/null
+++ b/drivers/clk/sprd/gate.h
@@ -0,0 +1,61 @@
+/*
+ * Spreadtrum gate clock driver
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _SPRD_GATE_H_
+#define _SPRD_GATE_H_
+
+#include "common.h"
+
+struct sprd_gate {
+ u32 enable_mask;
+ u16 flags;
+ u16 sc_offset;
+
+ struct sprd_clk_common common;
+};
+
+#define SPRD_SC_GATE_CLK_OPS(_struct, _name, _parent, _reg, _sc_offset, \
+ _enable_mask, _flags, _gate_flags, _ops) \
+ struct sprd_gate _struct = { \
+ .enable_mask = _enable_mask, \
+ .sc_offset = _sc_offset, \
+ .flags = _gate_flags, \
+ .common = { \
+ .regmap = NULL, \
+ .reg = _reg, \
+ .hw.init = CLK_HW_INIT(_name, \
+ _parent, \
+ _ops, \
+ _flags), \
+ } \
+ }
+
+#define SPRD_GATE_CLK(_struct, _name, _parent, _reg, \
+ _enable_mask, _flags, _gate_flags) \
+ SPRD_SC_GATE_CLK_OPS(_struct, _name, _parent, _reg, 0, \
+ _enable_mask, _flags, _gate_flags, \
+ &sprd_gate_ops)
+
+#define SPRD_SC_GATE_CLK(_struct, _name, _parent, _reg, _sc_offset, \
+ _enable_mask, _flags, _gate_flags) \
+ SPRD_SC_GATE_CLK_OPS(_struct, _name, _parent, _reg, _sc_offset, \
+ _enable_mask, _flags, _gate_flags, \
+ &sprd_sc_gate_ops)
+
+static inline struct sprd_gate *hw_to_sprd_gate(const struct clk_hw *hw)
+{
+ struct sprd_clk_common *common = hw_to_sprd_clk_common(hw);
+
+ return container_of(common, struct sprd_gate, common);
+}
+
+extern const struct clk_ops sprd_gate_ops;
+extern const struct clk_ops sprd_sc_gate_ops;
+
+#endif /* _SPRD_GATE_H_ */
--
2.7.4
^ permalink raw reply related
* [PATCH V5 04/12] clk: sprd: add mux clock support
From: Chunyan Zhang @ 2017-11-20 6:01 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, Rob Herring, Mark Rutland
Cc: Catalin Marinas, Will Deacon, linux-clk, linux-kernel, devicetree,
linux-arm-kernel, Arnd Bergmann, Mark Brown, Xiaolong Zhang,
Ben Li, Orson Zhai, Chunyan Zhang
In-Reply-To: <20171120060137.27380-1-chunyan.zhang@spreadtrum.com>
This patch adds clock multiplexor support for Spreadtrum platforms,
the mux clocks also can be found in sprd composite clocks, so
provides two helpers that can be reused later on.
Signed-off-by: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
---
drivers/clk/sprd/Makefile | 1 +
drivers/clk/sprd/mux.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/sprd/mux.h | 76 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 155 insertions(+)
create mode 100644 drivers/clk/sprd/mux.c
create mode 100644 drivers/clk/sprd/mux.h
diff --git a/drivers/clk/sprd/Makefile b/drivers/clk/sprd/Makefile
index 8cd5592..cee36b5 100644
--- a/drivers/clk/sprd/Makefile
+++ b/drivers/clk/sprd/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_SPRD_COMMON_CLK) += clk-sprd.o
clk-sprd-y += common.o
clk-sprd-y += gate.o
+clk-sprd-y += mux.o
diff --git a/drivers/clk/sprd/mux.c b/drivers/clk/sprd/mux.c
new file mode 100644
index 0000000..05ef9f0
--- /dev/null
+++ b/drivers/clk/sprd/mux.c
@@ -0,0 +1,78 @@
+/*
+ * Spreadtrum multiplexer clock driver
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+
+#include "mux.h"
+
+u8 sprd_mux_helper_get_parent(const struct sprd_clk_common *common,
+ const struct sprd_mux_ssel *mux)
+{
+ unsigned int reg;
+ u8 parent;
+ int num_parents;
+ int i;
+
+ sprd_regmap_read(common->regmap, common->reg, ®);
+ parent = reg >> mux->shift;
+ parent &= (1 << mux->width) - 1;
+
+ if (!mux->table)
+ return parent;
+
+ num_parents = clk_hw_get_num_parents(&common->hw);
+
+ for (i = 0; i < num_parents - 1; i++)
+ if (parent >= mux->table[i] && parent < mux->table[i + 1])
+ return i;
+
+ return num_parents - 1;
+}
+EXPORT_SYMBOL_GPL(sprd_mux_helper_get_parent);
+
+static u8 sprd_mux_get_parent(struct clk_hw *hw)
+{
+ struct sprd_mux *cm = hw_to_sprd_mux(hw);
+
+ return sprd_mux_helper_get_parent(&cm->common, &cm->mux);
+}
+
+int sprd_mux_helper_set_parent(const struct sprd_clk_common *common,
+ const struct sprd_mux_ssel *mux,
+ u8 index)
+{
+ unsigned int reg;
+
+ if (mux->table)
+ index = mux->table[index];
+
+ sprd_regmap_read(common->regmap, common->reg, ®);
+ reg &= ~GENMASK(mux->width + mux->shift - 1, mux->shift);
+ sprd_regmap_write(common->regmap, common->reg,
+ reg | (index << mux->shift));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(sprd_mux_helper_set_parent);
+
+static int sprd_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct sprd_mux *cm = hw_to_sprd_mux(hw);
+
+ return sprd_mux_helper_set_parent(&cm->common, &cm->mux, index);
+}
+
+const struct clk_ops sprd_mux_ops = {
+ .get_parent = sprd_mux_get_parent,
+ .set_parent = sprd_mux_set_parent,
+ .determine_rate = __clk_mux_determine_rate,
+};
+EXPORT_SYMBOL_GPL(sprd_mux_ops);
diff --git a/drivers/clk/sprd/mux.h b/drivers/clk/sprd/mux.h
new file mode 100644
index 0000000..683d63a
--- /dev/null
+++ b/drivers/clk/sprd/mux.h
@@ -0,0 +1,76 @@
+/*
+ * Spreadtrum multiplexer clock driver
+ *
+ * Copyright (C) 2017 Spreadtrum, Inc.
+ * Author: Chunyan Zhang <chunyan.zhang@spreadtrum.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _SPRD_MUX_H_
+#define _SPRD_MUX_H_
+
+#include "common.h"
+
+/**
+ * struct sprd_mux_ssel - Mux clock's source select bits in its register
+ * @shift: Bit offset of the divider in its register
+ * @width: Width of the divider field in its register
+ * @table: For some mux clocks, not all sources are used on some special
+ * chips, this matches the value of mux clock's register and the
+ * sources which are used for this mux clock
+ */
+struct sprd_mux_ssel {
+ u8 shift;
+ u8 width;
+ const u8 *table;
+};
+
+struct sprd_mux {
+ struct sprd_mux_ssel mux;
+ struct sprd_clk_common common;
+};
+
+#define _SPRD_MUX_CLK(_shift, _width, _table) \
+ { \
+ .shift = _shift, \
+ .width = _width, \
+ .table = _table, \
+ }
+
+#define SPRD_MUX_CLK_TABLE(_struct, _name, _parents, _table, \
+ _reg, _shift, _width, \
+ _flags) \
+ struct sprd_mux _struct = { \
+ .mux = _SPRD_MUX_CLK(_shift, _width, _table), \
+ .common = { \
+ .regmap = NULL, \
+ .reg = _reg, \
+ .hw.init = CLK_HW_INIT_PARENTS(_name, \
+ _parents, \
+ &sprd_mux_ops, \
+ _flags), \
+ } \
+ }
+
+#define SPRD_MUX_CLK(_struct, _name, _parents, _reg, \
+ _shift, _width, _flags) \
+ SPRD_MUX_CLK_TABLE(_struct, _name, _parents, NULL, \
+ _reg, _shift, _width, _flags)
+
+static inline struct sprd_mux *hw_to_sprd_mux(const struct clk_hw *hw)
+{
+ struct sprd_clk_common *common = hw_to_sprd_clk_common(hw);
+
+ return container_of(common, struct sprd_mux, common);
+}
+
+extern const struct clk_ops sprd_mux_ops;
+
+u8 sprd_mux_helper_get_parent(const struct sprd_clk_common *common,
+ const struct sprd_mux_ssel *mux);
+int sprd_mux_helper_set_parent(const struct sprd_clk_common *common,
+ const struct sprd_mux_ssel *mux,
+ u8 index);
+
+#endif /* _SPRD_MUX_H_ */
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox