* Re: [PATCH v3 04/10] media: i2c: imx290: Add support for 2 data lanes
From: Andrey Konovalov @ 2020-05-26 9:14 UTC (permalink / raw)
To: Sakari Ailus
Cc: mchehab, manivannan.sadhasivam, linux-media, linux-kernel,
devicetree, linux-arm-kernel, c.barrett, a.brela, peter.griffin
In-Reply-To: <20200526090127.GG8214@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thank you for the review
On 26.05.2020 12:01, Sakari Ailus wrote:
> Hi Andrey,
>
> On Sun, May 24, 2020 at 10:24:59PM +0300, Andrey Konovalov wrote:
>> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>
>> The IMX290 sensor can output frames with 2/4 CSI2 data lanes. This commit
>> adds support for 2 lane mode in addition to the 4 lane and also
>> configuring the data lane settings in the driver based on system
>> configuration.
>>
>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
>> ---
>> drivers/media/i2c/imx290.c | 133 ++++++++++++++++++++++++++++++++++---
>> 1 file changed, 124 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
>> index 7b1de1f0c8b7..a361c9ac8bd5 100644
>> --- a/drivers/media/i2c/imx290.c
>> +++ b/drivers/media/i2c/imx290.c
>> @@ -25,7 +25,18 @@
>> #define IMX290_STANDBY 0x3000
>> #define IMX290_REGHOLD 0x3001
>> #define IMX290_XMSTA 0x3002
>> +#define IMX290_FR_FDG_SEL 0x3009
>> #define IMX290_GAIN 0x3014
>> +#define IMX290_HMAX_LOW 0x301c
>> +#define IMX290_HMAX_HIGH 0x301d
>> +#define IMX290_PHY_LANE_NUM 0x3407
>> +#define IMX290_CSI_LANE_MODE 0x3443
>> +
>> +/* HMAX fields */
>> +#define IMX290_HMAX_2_1920 0x1130
>> +#define IMX290_HMAX_4_1920 0x0898
>> +#define IMX290_HMAX_2_720 0x19C8
>> +#define IMX290_HMAX_4_720 0x0CE4
>>
>> #define IMX290_DEFAULT_LINK_FREQ 445500000
>>
>> @@ -56,6 +67,7 @@ struct imx290 {
>> struct device *dev;
>> struct clk *xclk;
>> struct regmap *regmap;
>> + u8 nlanes;
>>
>> struct v4l2_subdev sd;
>> struct v4l2_fwnode_endpoint ep;
>> @@ -89,14 +101,11 @@ static const struct regmap_config imx290_regmap_config = {
>>
>> static const struct imx290_regval imx290_global_init_settings[] = {
>> { 0x3007, 0x00 },
>> - { 0x3009, 0x00 },
>> { 0x3018, 0x65 },
>> { 0x3019, 0x04 },
>> { 0x301a, 0x00 },
>> - { 0x3443, 0x03 },
>> { 0x3444, 0x20 },
>> { 0x3445, 0x25 },
>> - { 0x3407, 0x03 },
>> { 0x303a, 0x0c },
>> { 0x3040, 0x00 },
>> { 0x3041, 0x00 },
>> @@ -169,7 +178,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
>> { 0x3164, 0x1a },
>> { 0x3480, 0x49 },
>> /* data rate settings */
>> - { 0x3009, 0x01 },
>> { 0x3405, 0x10 },
>> { 0x3446, 0x57 },
>> { 0x3447, 0x00 },
>> @@ -187,8 +195,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
>> { 0x3453, 0x00 },
>> { 0x3454, 0x17 },
>> { 0x3455, 0x00 },
>> - { 0x301c, 0x98 },
>> - { 0x301d, 0x08 },
>> };
>>
>> static const struct imx290_regval imx290_720p_settings[] = {
>> @@ -210,7 +216,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
>> { 0x3164, 0x1a },
>> { 0x3480, 0x49 },
>> /* data rate settings */
>> - { 0x3009, 0x01 },
>> { 0x3405, 0x10 },
>> { 0x3446, 0x4f },
>> { 0x3447, 0x00 },
>> @@ -228,8 +233,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
>> { 0x3453, 0x00 },
>> { 0x3454, 0x17 },
>> { 0x3455, 0x00 },
>> - { 0x301c, 0xe4 },
>> - { 0x301d, 0x0c },
>> };
>>
>> static const struct imx290_regval imx290_10bit_settings[] = {
>> @@ -522,6 +525,25 @@ static int imx290_write_current_format(struct imx290 *imx290,
>> return 0;
>> }
>>
>> +static int imx290_set_hmax(struct imx290 *imx290, u32 val)
>> +{
>> + int ret;
>> +
>> + ret = imx290_write_reg(imx290, IMX290_HMAX_LOW, (val & 0xff));
>> + if (ret) {
>> + dev_err(imx290->dev, "Error setting HMAX register\n");
>> + return ret;
>> + }
>> +
>> + ret = imx290_write_reg(imx290, IMX290_HMAX_HIGH, ((val >> 8) & 0xff));
>> + if (ret) {
>> + dev_err(imx290->dev, "Error setting HMAX register\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /* Start streaming */
>> static int imx290_start_streaming(struct imx290 *imx290)
>> {
>> @@ -551,6 +573,40 @@ static int imx290_start_streaming(struct imx290 *imx290)
>> return ret;
>> }
>>
>> + switch (imx290->nlanes) {
>> + case 2:
>> + if (imx290->current_mode->width == 1920) {
>> + ret = imx290_set_hmax(imx290, IMX290_HMAX_2_1920);
>> + if (ret < 0)
>> + return ret;
>> + } else {
>> + ret = imx290_set_hmax(imx290, IMX290_HMAX_2_720);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + break;
>> + case 4:
>> + if (imx290->current_mode->width == 1920) {
>> + ret = imx290_set_hmax(imx290, IMX290_HMAX_4_1920);
>> + if (ret < 0)
>> + return ret;
>> + } else {
>> + ret = imx290_set_hmax(imx290, IMX290_HMAX_4_720);
>> + if (ret < 0)
>> + return ret;
>
> I think it'd be nicer to put this where the mode definitions are, to avoid
> scattering the configuration around the driver.
Would it be OK if I move this inside imx290_write_current_format()?
>> + }
>> +
>> + break;
>> + default:
>> + /*
>> + * We should never hit this since the data lane count is
>> + * validated in probe itself
>> + */
>> + dev_err(imx290->dev, "Lane configuration not supported\n");
>> + return -EINVAL;
>> + }
>> +
>> /* Apply customized values from user */
>> ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
>> if (ret) {
>> @@ -607,6 +663,49 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
>> imx290->supplies);
>> }
>>
>> +static int imx290_set_data_lanes(struct imx290 *imx290)
>> +{
>> + int ret = 0, laneval, frsel;
>> +
>> + switch (imx290->nlanes) {
>> + case 2:
>> + laneval = 0x01;
>> + frsel = 0x02;
>> + break;
>> + case 4:
>> + laneval = 0x03;
>> + frsel = 0x01;
>> + break;
>> + default:
>> + /*
>> + * We should never hit this since the data lane count is
>> + * validated in probe itself
>> + */
>> + dev_err(imx290->dev, "Lane configuration not supported\n");
>> + ret = -EINVAL;
>> + goto exit;
>> + }
>> +
>> + ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
>> + if (ret) {
>> + dev_err(imx290->dev, "Error setting Physical Lane number register\n");
>> + goto exit;
>> + }
>> +
>> + ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
>> + if (ret) {
>> + dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
>> + goto exit;
>> + }
>> +
>> + ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
>> + if (ret)
>> + dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
>> +
>> +exit:
>> + return ret;
>> +}
>> +
>> static int imx290_power_on(struct device *dev)
>> {
>> struct i2c_client *client = to_i2c_client(dev);
>> @@ -631,6 +730,9 @@ static int imx290_power_on(struct device *dev)
>> gpiod_set_value_cansleep(imx290->rst_gpio, 0);
>> usleep_range(30000, 31000);
>>
>> + /* Set data lane count */
>> + imx290_set_data_lanes(imx290);
>> +
>> return 0;
>> }
>>
>> @@ -703,6 +805,16 @@ static int imx290_probe(struct i2c_client *client)
>> goto free_err;
>> }
>>
>> + /* Get number of data lanes */
>
> While at it, could you set the PHY type in the V4L2 fwnode endpoint before
> parsing the data using v4l2_fwnode_endpoint_alloc_parse()?
This is currently done in "[PATCH v3 10/10] media: i2c: imx290: set bus_type
before calling v4l2_fwnode_endpoint_alloc_parse()" (along with some more
clean-ups for the probe()). I can merge the PHY type in the V4L2 fwnode endpoint
change into this patch.
Thanks,
Andrey
>> + imx290->nlanes = imx290->ep.bus.mipi_csi2.num_data_lanes;
>> + if (imx290->nlanes != 2 && imx290->nlanes != 4) {
>> + dev_err(dev, "Invalid data lanes: %d\n", imx290->nlanes);
>> + ret = -EINVAL;
>> + goto free_err;
>> + }
>> +
>> + dev_dbg(dev, "Using %u data lanes\n", imx290->nlanes);
>> +
>> if (!imx290->ep.nr_of_link_frequencies) {
>> dev_err(dev, "link-frequency property not found in DT\n");
>> ret = -EINVAL;
>> @@ -823,6 +935,9 @@ static int imx290_probe(struct i2c_client *client)
>> goto free_entity;
>> }
>>
>> + /* Set data lane count */
>> + imx290_set_data_lanes(imx290);
>> +
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>> pm_runtime_idle(dev);
>
^ permalink raw reply
* Re: [PATCH v3 05/10] media: i2c: imx290: Add configurable link frequency and pixel rate
From: Sakari Ailus @ 2020-05-26 9:12 UTC (permalink / raw)
To: Andrey Konovalov
Cc: mchehab, manivannan.sadhasivam, linux-media, linux-kernel,
devicetree, linux-arm-kernel, c.barrett, a.brela, peter.griffin
In-Reply-To: <20200524192505.20682-6-andrey.konovalov@linaro.org>
Hi Andrey,
On Sun, May 24, 2020 at 10:25:00PM +0300, Andrey Konovalov wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> IMX290 operates with multiple link frequency and pixel rate combinations.
> The initial driver used a single setting for both but since we now have
> the lane count support in place, let's add configurable link frequency
> and pixel rate.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
> drivers/media/i2c/imx290.c | 100 ++++++++++++++++++++++++-------------
> 1 file changed, 66 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index a361c9ac8bd5..e800557cf423 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -38,8 +38,6 @@
> #define IMX290_HMAX_2_720 0x19C8
> #define IMX290_HMAX_4_720 0x0CE4
>
> -#define IMX290_DEFAULT_LINK_FREQ 445500000
> -
> static const char * const imx290_supply_name[] = {
> "vdda",
> "vddd",
> @@ -56,8 +54,6 @@ struct imx290_regval {
> struct imx290_mode {
> u32 width;
> u32 height;
> - u32 pixel_rate;
> - u32 link_freq_index;
>
> const struct imx290_regval *data;
> u32 data_size;
> @@ -248,8 +244,13 @@ static const struct imx290_regval imx290_10bit_settings[] = {
> };
>
> /* supported link frequencies */
> -static const s64 imx290_link_freq[] = {
> - IMX290_DEFAULT_LINK_FREQ,
> +static const s64 imx290_link_freq_2lanes[] = {
> + 891000000, /* 1920x1080 - 2 lane */
> + 594000000, /* 1280x720 - 2 lane */
> +};
> +static const s64 imx290_link_freq_4lanes[] = {
> + 445500000, /* 1920x1080 - 4 lane */
> + 297000000, /* 1280x720 - 4 lane */
> };
>
> /* Mode configs */
> @@ -259,16 +260,12 @@ static const struct imx290_mode imx290_modes[] = {
> .height = 1080,
> .data = imx290_1080p_settings,
> .data_size = ARRAY_SIZE(imx290_1080p_settings),
> - .pixel_rate = 178200000,
> - .link_freq_index = 0,
> },
> {
> .width = 1280,
> .height = 720,
> .data = imx290_720p_settings,
> .data_size = ARRAY_SIZE(imx290_720p_settings),
> - .pixel_rate = 178200000,
> - .link_freq_index = 0,
> },
> };
>
> @@ -442,6 +439,32 @@ static int imx290_get_fmt(struct v4l2_subdev *sd,
> return 0;
> }
>
> +static u8 imx290_get_link_freq_index(struct imx290 *imx290)
> +{
> + const struct imx290_mode *cur_mode = imx290->current_mode;
> +
> + return (cur_mode->width == 1920) ? 0 : 1;
Could you use (imx290->current_mode - imx290_modes) / sizeof(*imx290_modes)
or something like that? It'd have fewer chances of breaking if new modes
are added.
> +}
> +
> +static s64 imx290_get_link_freq(struct imx290 *imx290)
> +{
> + u8 index = imx290_get_link_freq_index(imx290);
> +
> + if (imx290->nlanes == 4)
> + return imx290_link_freq_4lanes[index];
> + else
> + return imx290_link_freq_2lanes[index];
Or even better: store the link frequencies to the modes themselves. They
are a property of the modes after all.
> +}
> +
> +static u64 imx290_calc_pixel_rate(struct imx290 *imx290)
> +{
> + s64 link_freq = imx290_get_link_freq(imx290);
> + u8 nlanes = imx290->nlanes;
> +
> + /* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> + return (link_freq * 2 * nlanes / 10);
> +}
> +
> static int imx290_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_format *fmt)
> @@ -475,10 +498,14 @@ static int imx290_set_fmt(struct v4l2_subdev *sd,
> format = v4l2_subdev_get_try_format(sd, cfg, fmt->pad);
> } else {
> format = &imx290->current_format;
> - __v4l2_ctrl_s_ctrl(imx290->link_freq, mode->link_freq_index);
> - __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate, mode->pixel_rate);
> -
> imx290->current_mode = mode;
> +
> + if (imx290->link_freq)
> + __v4l2_ctrl_s_ctrl(imx290->link_freq,
> + imx290_get_link_freq_index(imx290));
> + if (imx290->pixel_rate)
> + __v4l2_ctrl_s_ctrl_int64(imx290->pixel_rate,
> + imx290_calc_pixel_rate(imx290));
> }
>
> *format = fmt->format;
> @@ -502,12 +529,11 @@ static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
> return 0;
> }
>
> -static int imx290_write_current_format(struct imx290 *imx290,
> - struct v4l2_mbus_framefmt *format)
> +static int imx290_write_current_format(struct imx290 *imx290)
> {
> int ret;
>
> - switch (format->code) {
> + switch (imx290->current_format.code) {
> case MEDIA_BUS_FMT_SRGGB10_1X10:
> ret = imx290_set_register_array(imx290, imx290_10bit_settings,
> ARRAY_SIZE(
> @@ -558,8 +584,8 @@ static int imx290_start_streaming(struct imx290 *imx290)
> return ret;
> }
>
> - /* Set current frame format */
> - ret = imx290_write_current_format(imx290, &imx290->current_format);
> + /* Apply the register values related to current frame format */
> + ret = imx290_write_current_format(imx290);
> if (ret < 0) {
> dev_err(imx290->dev, "Could not set frame format\n");
> return ret;
> @@ -821,12 +847,6 @@ static int imx290_probe(struct i2c_client *client)
> goto free_err;
> }
>
> - if (imx290->ep.link_frequencies[0] != IMX290_DEFAULT_LINK_FREQ) {
This check needs to be modified to correspond to the driver's new
capabilities, not removed.
> - dev_err(dev, "Unsupported link frequency\n");
> - ret = -EINVAL;
> - goto free_err;
> - }
> -
> /* Only CSI2 is supported for now */
> if (imx290->ep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> dev_err(dev, "Unsupported bus type, should be CSI2\n");
> @@ -879,23 +899,38 @@ static int imx290_probe(struct i2c_client *client)
>
> mutex_init(&imx290->lock);
>
> + /*
> + * Initialize the frame format. In particular, imx290->current_mode
> + * and imx290->bpp are set to defaults: imx290_calc_pixel_rate() call
> + * below relies on these fields.
> + */
> + imx290_entity_init_cfg(&imx290->sd, NULL);
> +
> v4l2_ctrl_handler_init(&imx290->ctrls, 3);
>
> v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> V4L2_CID_GAIN, 0, 72, 1, 0);
> - imx290->link_freq =
> - v4l2_ctrl_new_int_menu(&imx290->ctrls,
> - &imx290_ctrl_ops,
> - V4L2_CID_LINK_FREQ,
> - ARRAY_SIZE(imx290_link_freq) - 1,
> - 0, imx290_link_freq);
> + if (imx290->nlanes == 4)
> + imx290->link_freq =
> + v4l2_ctrl_new_int_menu(&imx290->ctrls,
> + &imx290_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(imx290_link_freq_4lanes) - 1,
> + 0, imx290_link_freq_4lanes);
> + else
> + imx290->link_freq =
> + v4l2_ctrl_new_int_menu(&imx290->ctrls,
> + &imx290_ctrl_ops,
> + V4L2_CID_LINK_FREQ,
> + ARRAY_SIZE(imx290_link_freq_2lanes) - 1,
> + 0, imx290_link_freq_2lanes);
> if (imx290->link_freq)
> imx290->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> imx290->pixel_rate = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> V4L2_CID_PIXEL_RATE, 1,
> INT_MAX, 1,
> - imx290_modes[0].pixel_rate);
> + imx290_calc_pixel_rate(imx290));
>
> imx290->sd.ctrl_handler = &imx290->ctrls;
>
> @@ -919,9 +954,6 @@ static int imx290_probe(struct i2c_client *client)
> goto free_ctrl;
> }
>
> - /* Initialize the frame format (this also sets imx290->current_mode) */
> - imx290_entity_init_cfg(&imx290->sd, NULL);
> -
> ret = v4l2_async_register_subdev(&imx290->sd);
> if (ret < 0) {
> dev_err(dev, "Could not register v4l2 device\n");
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v10 5/5] MIPS: Loongson64: Switch to generic PCI driver
From: Lorenzo Pieralisi @ 2020-05-26 9:12 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jiaxun Yang, linux-pci, Bjorn Helgaas, Rob Herring, Huacai Chen,
Paul Burton, devicetree, linux-kernel, linux-mips
In-Reply-To: <20200522223656.GA22313@alpha.franken.de>
On Sat, May 23, 2020 at 12:36:56AM +0200, Thomas Bogendoerfer wrote:
> On Fri, May 22, 2020 at 04:22:11PM +0100, Lorenzo Pieralisi wrote:
> > On Fri, May 22, 2020 at 04:25:50PM +0200, Thomas Bogendoerfer wrote:
> > > On Thu, May 14, 2020 at 09:16:41PM +0800, Jiaxun Yang wrote:
> > > > We can now enable generic PCI driver in Kconfig, and remove legacy
> > > > PCI driver code.
> > > >
> > > > Radeon vbios quirk is moved to the platform folder to fit the
> > > > new structure.
> > > >
> > > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > > > --
> > > > v9: Fix licenses tag
> > > > ---
> > > > arch/mips/Kconfig | 1 +
> > > > arch/mips/loongson64/Makefile | 2 +-
> > > > arch/mips/loongson64/vbios_quirk.c | 29 ++++++++
> > > > arch/mips/pci/Makefile | 1 -
> > > > arch/mips/pci/fixup-loongson3.c | 71 ------------------
> > > > arch/mips/pci/ops-loongson3.c | 116 -----------------------------
> > > > 6 files changed, 31 insertions(+), 189 deletions(-)
> > > > create mode 100644 arch/mips/loongson64/vbios_quirk.c
> > > > delete mode 100644 arch/mips/pci/fixup-loongson3.c
> > > > delete mode 100644 arch/mips/pci/ops-loongson3.c
> > >
> > > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> >
> > This patch (so the series) does not apply to v5.7-rc1 which is our
> > baseline. I reiterate the point, isn't it better to take the whole
> > series through the MIPS tree ?
>
> sounds better then
>
> > Failing that, the series has to
> > be rebased (or split differently so that it can be taken through
> > different trees), just let me know.
>
> so let's take via mips-next. So can I add your Acked-by to the
> first three patches ?
I have acked patch 2, you have all required ACKs to get this series via
the MIPS tree now, apologies for the delay.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine
From: Roger Lu @ 2020-05-26 9:12 UTC (permalink / raw)
To: Matthias Brugger
Cc: Enric Balletbo Serra, Kevin Hilman, Rob Herring, Nicolas Boichat,
Stephen Boyd, Mark Rutland, Nishanth Menon, Angus Lin,
devicetree@vger.kernel.org, Linux PM list, linux-kernel,
Xiaoqing Liu, YT Lee, Fan Chen,
moderated list:ARM/Mediatek SoC support, HenryC Chen,
Charles Yang, Linux ARM
In-Reply-To: <3b810588-ac4a-7fec-2163-38555dd83928@gmail.com>
Hi Matthias,
Thanks for the feedback.
On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote:
>
> On 22/05/2020 11:40, Roger Lu wrote:
> >
> > Hi Enric,
> >
> > On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote:
> >> Hi Roger,
> >>
> >> Thank you for your patch. I have the feeling that this driver is
> >> complex and difficult to follow and I am wondering if it wouldn't be
> >> better if you can send a version that simply adds basic functionality
> >> for now. Some comments below.
> >
> > Thanks for the advices. I'll submit SVS v9 with basic functionality
> > patch + step by step functionalities' patches.
> >
> >>
> >> Missatge de Roger Lu <roger.lu@mediatek.com> del dia dl., 18 de maig
> >> 2020 a les 11:25:
> >>>
> >>> The SVS (Smart Voltage Scaling) engine is a piece
> >>> of hardware which is used to calculate optimized
> >>> voltage values of several power domains,
> >>> e.g. CPU/GPU/CCI, according to chip process corner,
> >>> temperatures, and other factors. Then DVFS driver
> >>> could apply those optimized voltage values to reduce
> >>> power consumption.
> >>>
> >>> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
> >>> ---
> >>> drivers/power/avs/Kconfig | 10 +
> >>> drivers/power/avs/Makefile | 1 +
> >>> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++
> >>> include/linux/power/mtk_svs.h | 23 +
> >>> 4 files changed, 2153 insertions(+)
> >>> create mode 100644 drivers/power/avs/mtk_svs.c
> >>> create mode 100644 include/linux/power/mtk_svs.h
> >>>
> >>> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
> >>> index cdb4237bfd02..67089ac6040e 100644
> >>> --- a/drivers/power/avs/Kconfig
> >>> +++ b/drivers/power/avs/Kconfig
> >>> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN
> >>> Say y here to enable support io domains on Rockchip SoCs. It is
> >>> necessary for the io domain setting of the SoC to match the
> >>> voltage supplied by the regulators.
> >>> +
> >>> +config MTK_SVS
> >>> + bool "MediaTek Smart Voltage Scaling(SVS)"
> >>
> >> Can't be this a module? Why? In such case, you should use tristate option
> >
> > Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide
> > module option in config. If, somehow, SVS isn't needed, we suggest
> > CONFIG_MTK_SVS=n to be set.
> >
>
> The question here is if it needs to be probed before we probe the modules. If
> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS.
Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that
will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used
with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC
to enable SVS power feature. Anyway, back to Enric's question, I'll make
MTK_SVS become a tristate feature in the next patch. Thanks.
>
> >>
> >>> + depends on POWER_AVS && MTK_EFUSE && NVMEM
> >>> + help
> >>> + The SVS engine is a piece of hardware which is used to calculate
> >>> + optimized voltage values of several power domains, e.g.
> >>> + CPU clusters/GPU/CCI, according to chip process corner, temperatures,
> >>> + and other factors. Then DVFS driver could apply those optimized voltage
> >>> + values to reduce power consumption.
> >>> diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
> >>> index 9007d05853e2..231adf078582 100644
> >>> --- a/drivers/power/avs/Makefile
> >>> +++ b/drivers/power/avs/Makefile
> >>> @@ -2,3 +2,4 @@
> >>> obj-$(CONFIG_POWER_AVS_OMAP) += smartreflex.o
> >>> obj-$(CONFIG_QCOM_CPR) += qcom-cpr.o
> >>> obj-$(CONFIG_ROCKCHIP_IODOMAIN) += rockchip-io-domain.o
> >>> +obj-$(CONFIG_MTK_SVS) += mtk_svs.o
> >>
> >> Will this driver be SoC specific or the idea is to support different
> >> SoCs? If the answer to the first question is yes, please name the file
> >> with the SoC prefix (i.e mt8183_svs). However, If the answer to the
> >> second question is yes, make sure you prefix common
> >> functions/structs/defines with a generic prefix mtk_svs but use the
> >> SoC prefix for the ones you expect will be different between SoC, i.e
> >> mt8183_svs_. This helps the readability of the driver. Also, try to
> >> avoid too generic names.
> >
> > MTK_SVS is designed for supporting different MTK SoCs.Therefore, the answer is second
> > question and thanks for the heads-up.
> >
> >>
> >>> diff --git a/drivers/power/avs/mtk_svs.c b/drivers/power/avs/mtk_svs.c
> >>> new file mode 100644
> >>> index 000000000000..a4083b3ef175
> >>> --- /dev/null
> >>> +++ b/drivers/power/avs/mtk_svs.c
> >>> @@ -0,0 +1,2119 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>
> >> I suspect you want this only GPLv2 compliant. Use GPL-2.0-only
> >
> > OK. I'll use GPL-2.0-only.Thanks.
> >
> >>
> >>> +/*
> >>> + * Copyright (C) 2020 MediaTek Inc.
> >>> + */
> >>> +
> >>> +#define pr_fmt(fmt) "[mtk_svs] " fmt
> >>
> >> I don't see any reason to use pr_fmt in this driver. Use dev_*
> >> functions instead and remove the above.
> >
> > Ok. I will remove it. Thanks.
> >
> >>
> >>> +
> >>> +#include <linux/bits.h>
> >>> +#include <linux/clk.h>
> >>> +#include <linux/completion.h>
> >>> +#include <linux/init.h>
> >>> +#include <linux/interrupt.h>
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/kthread.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/mutex.h>
> >>> +#include <linux/nvmem-consumer.h>
> >>> +#include <linux/of_address.h>
> >>> +#include <linux/of_irq.h>
> >>> +#include <linux/of_platform.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <linux/pm_domain.h>
> >>> +#include <linux/pm_opp.h>
> >>> +#include <linux/pm_qos.h>
> >>> +#include <linux/pm_runtime.h>
> >>> +#include <linux/power/mtk_svs.h>
> >>> +#include <linux/proc_fs.h>
> >>> +#include <linux/regulator/consumer.h>
> >>> +#include <linux/reset.h>
> >>> +#include <linux/seq_file.h>
> >>> +#include <linux/slab.h>
> >>> +#include <linux/spinlock.h>
> >>> +#include <linux/thermal.h>
> >>> +#include <linux/uaccess.h>
> >>> +
> >>> +/* svs 1-line sw id */
> >>> +#define SVS_CPU_LITTLE BIT(0)
> >>> +#define SVS_CPU_BIG BIT(1)
> >>> +#define SVS_CCI BIT(2)
> >>> +#define SVS_GPU BIT(3)
> >>> +
> >>> +/* svs bank mode support */
> >>> +#define SVSB_MODE_ALL_DISABLE (0)
> >>
> >> nit: SVS_BMODE_?
> >
> > Oh. If we add bank wording like SVS_Bxxx, it might cause some confusion when B combines
> > with other words. So, I'll keep SVSB for SVS Bank representation.
> > E.g: SVS_BDC_SIGNED_BIT might lead to be explained differently ("SVS bank + DC_SIGNED_BIT" or "SVS + BDC_SIGNED_BIT")
> > - "SVS bank + DC_SIGNED_BIT" is what we want for naming SVS_BDC_SIGNED_BIT but it might be misunderstood.
> >
> >>
> >>> +#define SVSB_MODE_INIT01 BIT(1)
> >>> +#define SVSB_MODE_INIT02 BIT(2)
> >>> +#define SVSB_MODE_MON BIT(3)
> >>> +
> >>> +/* svs bank init01 condition */
> >>> +#define SVSB_INIT01_VOLT_IGNORE BIT(1)
> >>> +#define SVSB_INIT01_VOLT_INC_ONLY BIT(2)
> >>> +
> >>> +/* svs bank common setting */
> >>> +#define HIGH_TEMP_MAX (U32_MAX)
> >>
> >> nit: SVS_*
> >
> > ok. I will add SVS or SVSB when it refers to SVS BANK.
> >
> >>
> >>> +#define RUNCONFIG_DEFAULT (0x80000000)
> >>
> >> Btw, there is any public datasheet where I can see those addresses and
> >> registers and bit fields?
> >
> > Excuse us, there is no public datasheet. We can reply it on patchwork. Thanks.
> >
> >>
> >>> +#define DC_SIGNED_BIT (0x8000)
> >>> +#define INTEN_INIT0x (0x00005f01)
> >>> +#define INTEN_MONVOPEN (0x00ff0000)
> >>> +#define SVSEN_OFF (0x0)
> >>> +#define SVSEN_MASK (0x7)
> >>> +#define SVSEN_INIT01 (0x1)
> >>> +#define SVSEN_INIT02 (0x5)
> >>> +#define SVSEN_MON (0x2)
> >>> +#define INTSTS_MONVOP (0x00ff0000)
> >>> +#define INTSTS_COMPLETE (0x1)
> >>> +#define INTSTS_CLEAN (0x00ffffff)
> >>> +
> >>> +#define proc_fops_rw(name) \
> >>> + static int name ## _proc_open(struct inode *inode, \
> >>> + struct file *file) \
> >>> + { \
> >>> + return single_open(file, name ## _proc_show, \
> >>> + PDE_DATA(inode)); \
> >>> + } \
> >>> + static const struct proc_ops name ## _proc_fops = { \
> >>> + .proc_open = name ## _proc_open, \
> >>> + .proc_read = seq_read, \
> >>> + .proc_lseek = seq_lseek, \
> >>> + .proc_release = single_release, \
> >>> + .proc_write = name ## _proc_write, \
> >>> + }
> >>> +
> >>> +#define proc_fops_ro(name) \
> >>> + static int name ## _proc_open(struct inode *inode, \
> >>> + struct file *file) \
> >>> + { \
> >>> + return single_open(file, name ## _proc_show, \
> >>> + PDE_DATA(inode)); \
> >>> + } \
> >>> + static const struct proc_ops name ## _proc_fops = { \
> >>> + .proc_open = name ## _proc_open, \
> >>> + .proc_read = seq_read, \
> >>> + .proc_lseek = seq_lseek, \
> >>> + .proc_release = single_release, \
> >>> + }
> >>> +
> >>> +#define proc_entry(name) {__stringify(name), &name ## _proc_fops}
> >>> +
> >>
> >> /proc is usually the old way of exporting files to userspace, so
> >> unless you have a really good reason use sysfs instead, or even
> >> better, if it is only for debug purposes use debugfs. Also, you should
> >> document the entries in Documentation.
> >
> > Ok. I'll change it to debugfs and could you give us an example about entries in documentation?
> > We can follow them. Thanks.
> >
> >>
> >>> +static DEFINE_SPINLOCK(mtk_svs_lock);
> >>> +struct mtk_svs;
> >>> +
> >>> +enum svsb_phase {
> >>
> >> nit: mtk_svs_bphase?
> >
> > ditto
> >
> >>
> >>> + SVSB_PHASE_INIT01 = 0,
> >>
> >> nit: SVS_BPHASE_?
> >
> > ditto
> >
> >>
> >>> + SVSB_PHASE_INIT02,
> >>> + SVSB_PHASE_MON,
> >>> + SVSB_PHASE_ERROR,
> >>> +};
> >>> +
> >>> +enum reg_index {
> >>
> >> nit: svs_reg_index?
> >
> > OK. Thanks.
> >
> >>
> >>> + TEMPMONCTL0 = 0,
> >>> + TEMPMONCTL1,
> >>> + TEMPMONCTL2,
> >>> + TEMPMONINT,
> >>> + TEMPMONINTSTS,
> >>> + TEMPMONIDET0,
> >>> + TEMPMONIDET1,
> >>> + TEMPMONIDET2,
> >>> + TEMPH2NTHRE,
> >>> + TEMPHTHRE,
> >>> + TEMPCTHRE,
> >>> + TEMPOFFSETH,
> >>> + TEMPOFFSETL,
> >>> + TEMPMSRCTL0,
> >>> + TEMPMSRCTL1,
> >>> + TEMPAHBPOLL,
> >>> + TEMPAHBTO,
> >>> + TEMPADCPNP0,
> >>> + TEMPADCPNP1,
> >>> + TEMPADCPNP2,
> >>> + TEMPADCMUX,
> >>> + TEMPADCEXT,
> >>> + TEMPADCEXT1,
> >>> + TEMPADCEN,
> >>> + TEMPPNPMUXADDR,
> >>> + TEMPADCMUXADDR,
> >>> + TEMPADCEXTADDR,
> >>> + TEMPADCEXT1ADDR,
> >>> + TEMPADCENADDR,
> >>> + TEMPADCVALIDADDR,
> >>> + TEMPADCVOLTADDR,
> >>> + TEMPRDCTRL,
> >>> + TEMPADCVALIDMASK,
> >>> + TEMPADCVOLTAGESHIFT,
> >>> + TEMPADCWRITECTRL,
> >>> + TEMPMSR0,
> >>> + TEMPMSR1,
> >>> + TEMPMSR2,
> >>> + TEMPADCHADDR,
> >>> + TEMPIMMD0,
> >>> + TEMPIMMD1,
> >>> + TEMPIMMD2,
> >>> + TEMPMONIDET3,
> >>> + TEMPADCPNP3,
> >>> + TEMPMSR3,
> >>> + TEMPIMMD3,
> >>> + TEMPPROTCTL,
> >>> + TEMPPROTTA,
> >>> + TEMPPROTTB,
> >>> + TEMPPROTTC,
> >>> + TEMPSPARE0,
> >>> + TEMPSPARE1,
> >>> + TEMPSPARE2,
> >>> + TEMPSPARE3,
> >>> + TEMPMSR0_1,
> >>> + TEMPMSR1_1,
> >>> + TEMPMSR2_1,
> >>> + TEMPMSR3_1,
> >>> + DESCHAR,
> >>> + TEMPCHAR,
> >>> + DETCHAR,
> >>> + AGECHAR,
> >>> + DCCONFIG,
> >>> + AGECONFIG,
> >>> + FREQPCT30,
> >>> + FREQPCT74,
> >>> + LIMITVALS,
> >>> + VBOOT,
> >>> + DETWINDOW,
> >>> + CONFIG,
> >>> + TSCALCS,
> >>> + RUNCONFIG,
> >>> + SVSEN,
> >>> + INIT2VALS,
> >>> + DCVALUES,
> >>> + AGEVALUES,
> >>> + VOP30,
> >>> + VOP74,
> >>> + TEMP,
> >>> + INTSTS,
> >>> + INTSTSRAW,
> >>> + INTEN,
> >>> + CHKINT,
> >>> + CHKSHIFT,
> >>> + STATUS,
> >>> + VDESIGN30,
> >>> + VDESIGN74,
> >>> + DVT30,
> >>> + DVT74,
> >>> + AGECOUNT,
> >>> + SMSTATE0,
> >>> + SMSTATE1,
> >>> + CTL0,
> >>> + DESDETSEC,
> >>> + TEMPAGESEC,
> >>> + CTRLSPARE0,
> >>> + CTRLSPARE1,
> >>> + CTRLSPARE2,
> >>> + CTRLSPARE3,
> >>> + CORESEL,
> >>> + THERMINTST,
> >>> + INTST,
> >>> + THSTAGE0ST,
> >>> + THSTAGE1ST,
> >>> + THSTAGE2ST,
> >>> + THAHBST0,
> >>> + THAHBST1,
> >>> + SPARE0,
> >>> + SPARE1,
> >>> + SPARE2,
> >>> + SPARE3,
> >>> + THSLPEVEB,
> >>> + reg_num,
> >>> +};
> >>> +
> >>> +static const u32 svs_regs_v2[] = {
> >>
> >> Is this SoC specific or shared between SoCs?
> >
> > Shared between SoCs. Some SVS in MTK SoCs use v2 register map.
> >
>
> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep?
1. MT8173 IC uses v1 register map.
2. Yes, I'll keep v2 postfix.
>
> >>
> >>> + [TEMPMONCTL0] = 0x000,
> >>> + [TEMPMONCTL1] = 0x004,
> >>> + [TEMPMONCTL2] = 0x008,
> >>> + [TEMPMONINT] = 0x00c,
> >>> + [TEMPMONINTSTS] = 0x010,
> >>> + [TEMPMONIDET0] = 0x014,
> >>> + [TEMPMONIDET1] = 0x018,
> >>> + [TEMPMONIDET2] = 0x01c,
> >>> + [TEMPH2NTHRE] = 0x024,
> >>> + [TEMPHTHRE] = 0x028,
> >>> + [TEMPCTHRE] = 0x02c,
> >>> + [TEMPOFFSETH] = 0x030,
> >>> + [TEMPOFFSETL] = 0x034,
> >>> + [TEMPMSRCTL0] = 0x038,
> >>> + [TEMPMSRCTL1] = 0x03c,
> >>> + [TEMPAHBPOLL] = 0x040,
> >>> + [TEMPAHBTO] = 0x044,
> >>> + [TEMPADCPNP0] = 0x048,
> >>> + [TEMPADCPNP1] = 0x04c,
> >>> + [TEMPADCPNP2] = 0x050,
> >>> + [TEMPADCMUX] = 0x054,
> >>> + [TEMPADCEXT] = 0x058,
> >>> + [TEMPADCEXT1] = 0x05c,
> >>> + [TEMPADCEN] = 0x060,
> >>> + [TEMPPNPMUXADDR] = 0x064,
> >>> + [TEMPADCMUXADDR] = 0x068,
> >>> + [TEMPADCEXTADDR] = 0x06c,
> >>> + [TEMPADCEXT1ADDR] = 0x070,
> >>> + [TEMPADCENADDR] = 0x074,
> >>> + [TEMPADCVALIDADDR] = 0x078,
> >>> + [TEMPADCVOLTADDR] = 0x07c,
> >>> + [TEMPRDCTRL] = 0x080,
> >>> + [TEMPADCVALIDMASK] = 0x084,
> >>> + [TEMPADCVOLTAGESHIFT] = 0x088,
> >>> + [TEMPADCWRITECTRL] = 0x08c,
> >>> + [TEMPMSR0] = 0x090,
> >>> + [TEMPMSR1] = 0x094,
> >>> + [TEMPMSR2] = 0x098,
> >>> + [TEMPADCHADDR] = 0x09c,
> >>> + [TEMPIMMD0] = 0x0a0,
> >>> + [TEMPIMMD1] = 0x0a4,
> >>> + [TEMPIMMD2] = 0x0a8,
> >>> + [TEMPMONIDET3] = 0x0b0,
> >>> + [TEMPADCPNP3] = 0x0b4,
> >>> + [TEMPMSR3] = 0x0b8,
> >>> + [TEMPIMMD3] = 0x0bc,
> >>> + [TEMPPROTCTL] = 0x0c0,
> >>> + [TEMPPROTTA] = 0x0c4,
> >>> + [TEMPPROTTB] = 0x0c8,
> >>> + [TEMPPROTTC] = 0x0cc,
> >>> + [TEMPSPARE0] = 0x0f0,
> >>> + [TEMPSPARE1] = 0x0f4,
> >>> + [TEMPSPARE2] = 0x0f8,
> >>> + [TEMPSPARE3] = 0x0fc,
> >>> + [TEMPMSR0_1] = 0x190,
> >>> + [TEMPMSR1_1] = 0x194,
> >>> + [TEMPMSR2_1] = 0x198,
> >>> + [TEMPMSR3_1] = 0x1b8,
> >>> + [DESCHAR] = 0xc00,
> >>> + [TEMPCHAR] = 0xc04,
> >>> + [DETCHAR] = 0xc08,
> >>> + [AGECHAR] = 0xc0c,
> >>> + [DCCONFIG] = 0xc10,
> >>> + [AGECONFIG] = 0xc14,
> >>> + [FREQPCT30] = 0xc18,
> >>> + [FREQPCT74] = 0xc1c,
> >>> + [LIMITVALS] = 0xc20,
> >>> + [VBOOT] = 0xc24,
> >>> + [DETWINDOW] = 0xc28,
> >>> + [CONFIG] = 0xc2c,
> >>> + [TSCALCS] = 0xc30,
> >>> + [RUNCONFIG] = 0xc34,
> >>> + [SVSEN] = 0xc38,
> >>> + [INIT2VALS] = 0xc3c,
> >>> + [DCVALUES] = 0xc40,
> >>> + [AGEVALUES] = 0xc44,
> >>> + [VOP30] = 0xc48,
> >>> + [VOP74] = 0xc4c,
> >>> + [TEMP] = 0xc50,
> >>> + [INTSTS] = 0xc54,
> >>> + [INTSTSRAW] = 0xc58,
> >>> + [INTEN] = 0xc5c,
> >>> + [CHKINT] = 0xc60,
> >>> + [CHKSHIFT] = 0xc64,
> >>> + [STATUS] = 0xc68,
> >>> + [VDESIGN30] = 0xc6c,
> >>> + [VDESIGN74] = 0xc70,
> >>> + [DVT30] = 0xc74,
> >>> + [DVT74] = 0xc78,
> >>> + [AGECOUNT] = 0xc7c,
> >>> + [SMSTATE0] = 0xc80,
> >>> + [SMSTATE1] = 0xc84,
> >>> + [CTL0] = 0xc88,
> >>> + [DESDETSEC] = 0xce0,
> >>> + [TEMPAGESEC] = 0xce4,
> >>> + [CTRLSPARE0] = 0xcf0,
> >>> + [CTRLSPARE1] = 0xcf4,
> >>> + [CTRLSPARE2] = 0xcf8,
> >>> + [CTRLSPARE3] = 0xcfc,
> >>> + [CORESEL] = 0xf00,
> >>> + [THERMINTST] = 0xf04,
> >>> + [INTST] = 0xf08,
> >>> + [THSTAGE0ST] = 0xf0c,
> >>> + [THSTAGE1ST] = 0xf10,
> >>> + [THSTAGE2ST] = 0xf14,
> >>> + [THAHBST0] = 0xf18,
> >>> + [THAHBST1] = 0xf1c,
> >>> + [SPARE0] = 0xf20,
> >>> + [SPARE1] = 0xf24,
> >>> + [SPARE2] = 0xf28,
> >>> + [SPARE3] = 0xf2c,
> >>> + [THSLPEVEB] = 0xf30,
> >>> +};
> >>> +
> >>> +struct thermal_parameter {
> >>
> >> In general, not only in this struct, would be good have some
> >> documentation to have a better undestanding of the fields. That makes
> >> the job of the reviewer a bit easier.
> >
> > Ok. Could you share a documentation example to us? We'll share the
> > information as much as we can. Thanks a lot.
> >
>
> you should find that in all drivers, eg:
> https://elixir.bootlin.com/linux/latest/source/drivers/soc/mediatek/mtk-scpsys.c#L111
No problem Sir. Thanks for showing a direction to me. I'll take a look
at it.
>
> Regards,
> Matthias
^ permalink raw reply
* Re: [PATCH v10 2/5] PCI: Add Loongson PCI Controller support
From: Lorenzo Pieralisi @ 2020-05-26 9:10 UTC (permalink / raw)
To: Jiaxun Yang
Cc: linux-pci, Rob Herring, Bjorn Helgaas, Rob Herring,
Thomas Bogendoerfer, Huacai Chen, Paul Burton, devicetree,
linux-kernel, linux-mips
In-Reply-To: <20200514131650.3587281-2-jiaxun.yang@flygoat.com>
On Thu, May 14, 2020 at 09:16:38PM +0800, Jiaxun Yang wrote:
> This controller can be found on Loongson-2K SoC, Loongson-3
> systems with RS780E/LS7A PCH.
>
> The RS780E part of code was previously located at
> arch/mips/pci/ops-loongson3.c and now it can use generic PCI
> driver implementation.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> --
> v2:
> - Clean up according to rob's suggestions
> - Claim that it can't work as a module
> v3:
> - Fix a typo
> v4:
> - More clean-ups: Drop flag check, use devfn
> v7:
> - Fix ordering according to huacai's suggestion
> v8:
> - Style clean and bugfix according to Bjorn's review
> v9:
> - Confirm with vendor about the details on system_bus BARs
> - Further cleanups
> - I think assume root_bus as 0 simplify our code a lot, as that's
> always valid to the hardware, as we're not ecam or dwc, we can't
> easily get host bus_nr via cfg, adding that will cause unnecessary
> overhead.
>
> v10:
> - Enhance comments about system_bus quirk and host bus quirk.
> ---
> drivers/pci/controller/Kconfig | 10 ++
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pci-loongson.c | 249 ++++++++++++++++++++++++++
> 3 files changed, 260 insertions(+)
> create mode 100644 drivers/pci/controller/pci-loongson.c
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 91bfdb784829..ae36edb1d7db 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -258,6 +258,16 @@ config PCI_HYPERV_INTERFACE
> The Hyper-V PCI Interface is a helper driver allows other drivers to
> have a common interface with the Hyper-V PCI frontend driver.
>
> +config PCI_LOONGSON
> + bool "LOONGSON PCI Controller"
> + depends on MACH_LOONGSON64 || COMPILE_TEST
> + depends on OF
> + depends on PCI_QUIRKS
> + default MACH_LOONGSON64
> + help
> + Say Y here if you want to enable PCI controller support on
> + Loongson systems.
> +
> source "drivers/pci/controller/dwc/Kconfig"
> source "drivers/pci/controller/mobiveil/Kconfig"
> source "drivers/pci/controller/cadence/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index 158c59771824..fbac4b0190a0 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
> obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
> obj-$(CONFIG_VMD) += vmd.o
> obj-$(CONFIG_PCIE_BRCMSTB) += pcie-brcmstb.o
> +obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
> obj-y += dwc/
> obj-y += mobiveil/
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> new file mode 100644
> index 000000000000..4879377b86f3
> --- /dev/null
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -0,0 +1,249 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Loongson PCI Host Controller Driver
> + *
> + * Copyright (C) 2020 Jiaxun Yang <jiaxun.yang@flygoat.com>
> + */
> +
> +#include <linux/of_device.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +
> +#include "../pci.h"
> +
> +/* Device IDs */
> +#define DEV_PCIE_PORT_0 0x7a09
> +#define DEV_PCIE_PORT_1 0x7a19
> +#define DEV_PCIE_PORT_2 0x7a29
> +
> +#define DEV_LS2K_APB 0x7a02
> +#define DEV_LS7A_CONF 0x7a10
> +#define DEV_LS7A_LPC 0x7a0c
> +
> +#define FLAG_CFG0 BIT(0)
> +#define FLAG_CFG1 BIT(1)
> +#define FLAG_DEV_FIX BIT(2)
> +
> +struct loongson_pci {
> + void __iomem *cfg0_base;
> + void __iomem *cfg1_base;
> + struct platform_device *pdev;
> + u32 flags;
> +};
> +
> +/* Fixup wrong class code in PCIe bridges */
> +static void bridge_class_quirk(struct pci_dev *dev)
> +{
> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_0, bridge_class_quirk);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_1, bridge_class_quirk);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_2, bridge_class_quirk);
> +
> +static void system_bus_quirk(struct pci_dev *pdev)
> +{
> + u16 tmp;
> +
> + /*
> + * The address space consumed by these devices is outside the
> + * resources of the host bridge.
> + */
> + pdev->mmio_always_on = 1;
> + pdev->non_compliant_bars = 1;
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS2K_APB, system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_CONF, system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_LPC, system_bus_quirk);
> +
> +static void loongson_mrrs_quirk(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> + { 0, },
> + };
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> + /*
> + * Some Loongson PCIe ports have a h/w limitation of
> + * 256 bytes maximum read request size. They can't handle
> + * anything larger than this. So force this limit on
> + * any devices attached under these ports.
> + */
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(dev) > 256) {
> + pci_info(dev, "limiting MRRS to 256\n");
> + pcie_set_readrq(dev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +
> +static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> + unsigned int devfn, int where)
> +{
> + unsigned long addroff = 0x0;
> +
> + if (bus != 0)
> + addroff |= BIT(28); /* Type 1 Access */
> + addroff |= (where & 0xff) | ((where & 0xf00) << 16);
> + addroff |= (bus << 16) | (devfn << 8);
> + return priv->cfg1_base + addroff;
> +}
> +
> +static void __iomem *cfg0_map(struct loongson_pci *priv, int bus,
> + unsigned int devfn, int where)
> +{
> + unsigned long addroff = 0x0;
> +
> + if (bus != 0)
> + addroff |= BIT(24); /* Type 1 Access */
> + addroff |= (bus << 16) | (devfn << 8) | where;
> + return priv->cfg0_base + addroff;
> +}
> +
> +void __iomem *pci_loongson_map_bus(struct pci_bus *bus, unsigned int devfn,
> + int where)
> +{
> + unsigned char busnum = bus->number;
> + struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
> + struct loongson_pci *priv = pci_host_bridge_priv(bridge);
> +
> + /*
> + * Do not read more than one device on the bus other than
> + * the host bus. For our hardware the root bus is always bus 0.
> + */
> + if (priv->flags & FLAG_DEV_FIX && busnum != 0 &&
> + PCI_SLOT(devfn) > 0)
> + return NULL;
> +
> + /* CFG0 can only access standard space */
> + if (where < PCI_CFG_SPACE_SIZE && priv->cfg0_base)
> + return cfg0_map(priv, busnum, devfn, where);
> +
> + /* CFG1 can access extended space */
> + if (where < PCI_CFG_SPACE_EXP_SIZE && priv->cfg1_base)
> + return cfg1_map(priv, busnum, devfn, where);
> +
> + return NULL;
> +}
> +
> +static int loongson_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> +{
> + int irq;
> + u8 val;
> +
> + irq = of_irq_parse_and_map_pci(dev, slot, pin);
> + if (irq > 0)
> + return irq;
> +
> + /* Care i8259 legacy systems */
> + pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &val);
> + /* i8259 only have 15 IRQs */
> + if (val > 15)
> + return 0;
> +
> + return val;
> +}
> +
> +/* H/w only accept 32-bit PCI operations */
> +static struct pci_ops loongson_pci_ops = {
> + .map_bus = pci_loongson_map_bus,
> + .read = pci_generic_config_read32,
> + .write = pci_generic_config_write32,
> +};
> +
> +static const struct of_device_id loongson_pci_of_match[] = {
> + { .compatible = "loongson,ls2k-pci",
> + .data = (void *)(FLAG_CFG0 | FLAG_CFG1 | FLAG_DEV_FIX), },
> + { .compatible = "loongson,ls7a-pci",
> + .data = (void *)(FLAG_CFG0 | FLAG_CFG1 | FLAG_DEV_FIX), },
> + { .compatible = "loongson,rs780e-pci",
> + .data = (void *)(FLAG_CFG0), },
> + {}
> +};
> +
> +static int loongson_pci_probe(struct platform_device *pdev)
> +{
> + struct loongson_pci *priv;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->of_node;
> + struct pci_host_bridge *bridge;
> + struct resource *regs;
> + int err;
> +
> + if (!node)
> + return -ENODEV;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*priv));
> + if (!bridge)
> + return -ENODEV;
> +
> + priv = pci_host_bridge_priv(bridge);
> + priv->pdev = pdev;
> + priv->flags = (unsigned long)of_device_get_match_data(dev);
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs) {
> + dev_err(dev, "missing mem resources for cfg0\n");
> + return -EINVAL;
> + }
> +
> + priv->cfg0_base = devm_pci_remap_cfg_resource(dev, regs);
> + if (IS_ERR(priv->cfg0_base))
> + return PTR_ERR(priv->cfg0_base);
> +
> + /* CFG1 is optional */
> + if (priv->flags & FLAG_CFG1) {
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!regs)
> + dev_info(dev, "missing mem resource for cfg1\n");
> + else {
> + priv->cfg1_base = devm_pci_remap_cfg_resource(dev, regs);
> + if (IS_ERR(priv->cfg1_base))
> + priv->cfg1_base = NULL;
> + }
> + }
> +
> + err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> + &bridge->dma_ranges, NULL);
> + if (err) {
> + dev_err(dev, "failed to get bridge resources\n");
> + return err;
> + }
> +
> + bridge->dev.parent = dev;
> + bridge->sysdata = priv;
> + bridge->ops = &loongson_pci_ops;
> + bridge->map_irq = loongson_map_irq;
> +
> + err = pci_host_probe(bridge);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static struct platform_driver loongson_pci_driver = {
> + .driver = {
> + .name = "loongson-pci",
> + .of_match_table = loongson_pci_of_match,
> + },
> + .probe = loongson_pci_probe,
> +};
> +builtin_platform_driver(loongson_pci_driver);
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH v5 6/8] drm/panel: Add ilitek ili9341 panel driver
From: dillon min @ 2020-05-26 9:08 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Noralf Trønnes, Linus Walleij, Mark Brown, devicetree,
linux-arm Mailing List, Linux Kernel Mailing List, linux-spi,
linux-stm32, dri-devel, linux-clk, Sam Ravnborg
In-Reply-To: <CAHp75VebSZa6mwAETnM0t42RQCp4iM6_SNjmy3TB48ixsGKV8g@mail.gmail.com>
Hi Andy,
Thanks for input.
On Tue, May 26, 2020 at 3:46 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, May 25, 2020 at 6:46 AM <dillon.minfei@gmail.com> wrote:
> >
> > From: dillon min <dillon.minfei@gmail.com>
> >
> > This driver combine tiny/ili9341.c mipi_dbi_interface driver
> > with mipi_dpi_interface driver, can support ili9341 with serial
> > mode or parallel rgb interface mode by register configuration.
>
> Noralf told once that this driver should be unified with mi0283qt.c.
>
> So, what should we do here?
>
> --
> With Best Regards,
> Andy Shevchenko
from sam's suggestion, we can't setup two drivers to support one panel
in the tree. so, i copy the mipi dbi part from tiny/ili9341.c. to this driver
from register settings and dts binding is keep the same to tiny/ili9341.c.
so, in my opinion if tiny/ili9341.c is unified with mi0283qt.c, this
driver should be
too.
thanks.
best regards,
Dillon,
^ permalink raw reply
* [PATCH v12 1/6] dt-bindings: mfd: add document bindings for mp2629
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
Add device tree binding information for mp2629 mfd driver.
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
.../devicetree/bindings/mfd/mps,mp2629.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
diff --git a/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
new file mode 100644
index 000000000000..f91acc42d652
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/mps,mp2629.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MP2629 Battery Charger PMIC from Monolithic Power System.
+
+maintainers:
+ - Saravanan Sekar <sravanhome@gmail.com>
+
+description: |
+ MP2629 is a PMIC providing battery charging and power supply for smartphones,
+ wireless camera and portable devices. Chip is controlled over I2C.
+
+ The battery charge management device handles battery charger controller and
+ ADC IIO device for battery, system voltage
+
+properties:
+ compatible:
+ const: mps,mp2629
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+ description:
+ The first cell is the IRQ number, the second cell is the trigger type.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-controller
+ - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/input/linux-event-codes.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic@4b {
+ compatible = "mps,mp2629";
+ reg = <0x4b>;
+
+ interrupt-controller;
+ interrupt-parent = <&gpio2>;
+ #interrupt-cells = <2>;
+ interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH v12 2/6] mfd: mp2629: Add support for mps battery charger
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
mp2629 is a highly-integrated switching-mode battery charge management
device for single-cell Li-ion or Li-polymer battery.
Add MFD core enables chip access for ADC driver for battery readings,
and a power supply battery-charger driver
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
drivers/mfd/Kconfig | 9 +++++
drivers/mfd/Makefile | 2 +
drivers/mfd/mp2629.c | 79 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/mp2629.h | 17 ++++++++
4 files changed, 107 insertions(+)
create mode 100644 drivers/mfd/mp2629.c
create mode 100644 include/linux/mfd/mp2629.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3c547ed575e6..fbf287098970 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C
help
Select this if your MC13xxx is connected via an I2C bus.
+config MFD_MP2629
+ tristate "Monolithic Power Systems MP2629 ADC and Battery charger"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Select this option to enable support for Monolithic Power Systems
+ battery charger. This provides ADC, thermal and battery charger power
+ management functions.
+
config MFD_MXS_LRADC
tristate "Freescale i.MX23/i.MX28 LRADC"
depends on ARCH_MXS || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10cbf0f..d6c210f96d02 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -170,6 +170,8 @@ obj-$(CONFIG_MFD_MAX8925) += max8925.o
obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o
obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o
+obj-$(CONFIG_MFD_MP2629) += mp2629.o
+
pcf50633-objs := pcf50633-core.o pcf50633-irq.o
obj-$(CONFIG_MFD_PCF50633) += pcf50633.o
obj-$(CONFIG_PCF50633_ADC) += pcf50633-adc.o
diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c
new file mode 100644
index 000000000000..16840ec5fd1c
--- /dev/null
+++ b/drivers/mfd/mp2629.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * MP2629 parent driver for ADC and battery charger
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+static const struct mfd_cell mp2629_cell[] = {
+ {
+ .name = "mp2629_adc",
+ .of_compatible = "mps,mp2629_adc",
+ },
+ {
+ .name = "mp2629_charger",
+ .of_compatible = "mps,mp2629_charger",
+ }
+};
+
+static const struct regmap_config mp2629_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x17,
+};
+
+static int mp2629_probe(struct i2c_client *client)
+{
+ struct mp2629_data *ddata;
+ int ret;
+
+ ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ ddata->dev = &client->dev;
+ i2c_set_clientdata(client, ddata);
+
+ ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config);
+ if (IS_ERR(ddata->regmap)) {
+ dev_err(ddata->dev, "Failed to allocate regmap\n");
+ return PTR_ERR(ddata->regmap);
+ }
+
+ ret = devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO, mp2629_cell,
+ ARRAY_SIZE(mp2629_cell), NULL, 0, NULL);
+ if (ret)
+ dev_err(ddata->dev, "Failed to register sub-devices %d\n", ret);
+
+ return ret;
+}
+
+static const struct of_device_id mp2629_of_match[] = {
+ { .compatible = "mps,mp2629"},
+ { }
+};
+MODULE_DEVICE_TABLE(of, mp2629_of_match);
+
+static struct i2c_driver mp2629_driver = {
+ .driver = {
+ .name = "mp2629",
+ .of_match_table = mp2629_of_match,
+ },
+ .probe_new = mp2629_probe,
+};
+module_i2c_driver(mp2629_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 Battery charger parent driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
new file mode 100644
index 000000000000..baaeeaf82949
--- /dev/null
+++ b/include/linux/mfd/mp2629.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2020 Monolithic Power Systems, Inc
+ */
+
+#ifndef __MP2629_H__
+#define __MP2629_H__
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+struct mp2629_data {
+ struct device *dev;
+ struct regmap *regmap;
+};
+
+#endif
--
2.17.1
^ permalink raw reply related
* [PATCH v12 6/6] MAINTAINERS: Add entry for mp2629 Battery Charger driver
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
Add MAINTAINERS entry for Monolithic Power Systems mp2629 Charger driver.
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
MAINTAINERS | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 32a95d162f06..0f82d5a7a614 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11358,10 +11358,15 @@ F: drivers/tty/mxser.*
MONOLITHIC POWER SYSTEM PMIC DRIVER
M: Saravanan Sekar <sravanhome@gmail.com>
S: Maintained
+F: Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
F: Documentation/devicetree/bindings/regulator/mps,mp*.yaml
+F: drivers/iio/adc/mp2629_adc.c
+F: drivers/mfd/mp2629.c
+F: drivers/power/supply/mp2629_charger.c
F: drivers/regulator/mp5416.c
F: drivers/regulator/mpq7920.c
F: drivers/regulator/mpq7920.h
+F: include/linux/mfd/mp2629.h
MR800 AVERMEDIA USB FM RADIO DRIVER
M: Alexey Klimov <klimov.linux@gmail.com>
--
2.17.1
^ permalink raw reply related
* [PATCH v12 5/6] power: supply: mp2629: Add impedance compensation config
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
Allows the user to compensate the intrinsic resistance of the battery
to accelerate the charging cycle.
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
Documentation/ABI/testing/sysfs-class-power-mp2629 | 8 ++++++++
1 file changed, 8 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-power-mp2629
diff --git a/Documentation/ABI/testing/sysfs-class-power-mp2629 b/Documentation/ABI/testing/sysfs-class-power-mp2629
new file mode 100644
index 000000000000..327a07e22805
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-power-mp2629
@@ -0,0 +1,8 @@
+What: /sys/class/power_supply/mp2629_battery/batt_impedance_compen
+Date: April 2020
+KernelVersion: 5.7
+Description:
+ Represents a battery impedance compensation to accelerate charging.
+
+ Access: Read, Write
+ Valid values: Represented in milli-ohms. Valid range is [0, 140].
--
2.17.1
^ permalink raw reply related
* [PATCH v12 4/6] power: supply: Add support for mps mp2629 battery charger
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
The mp2629 provides switching-mode battery charge management for
single-cell Li-ion or Li-polymer battery. Driver supports the
access/control input source and battery charging parameters.
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
drivers/power/supply/Kconfig | 10 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/mp2629_charger.c | 669 ++++++++++++++++++++++++++
3 files changed, 680 insertions(+)
create mode 100644 drivers/power/supply/mp2629_charger.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index f3424fdce341..a46643f5e2fb 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -541,6 +541,16 @@ config CHARGER_MAX8998
Say Y to enable support for the battery charger control sysfs and
platform data of MAX8998/LP3974 PMICs.
+config CHARGER_MP2629
+ tristate "Monolithic power system MP2629 Battery charger"
+ depends on MFD_MP2629
+ depends on MP2629_ADC
+ depends on IIO
+ help
+ Select this option to enable support for Monolithic power system
+ Battery charger. This driver provides Battery charger power management
+ functions on the systems.
+
config CHARGER_QCOM_SMBB
tristate "Qualcomm Switch-Mode Battery Charger and Boost"
depends on MFD_SPMI_PMIC || COMPILE_TEST
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6c7da920ea83..41cb64f09e49 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_CHARGER_MAX77650) += max77650-charger.o
obj-$(CONFIG_CHARGER_MAX77693) += max77693_charger.o
obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
+obj-$(CONFIG_CHARGER_MP2629) += mp2629_charger.o
obj-$(CONFIG_CHARGER_QCOM_SMBB) += qcom_smbb.o
obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
diff --git a/drivers/power/supply/mp2629_charger.c b/drivers/power/supply/mp2629_charger.c
new file mode 100644
index 000000000000..bdf924b73e47
--- /dev/null
+++ b/drivers/power/supply/mp2629_charger.c
@@ -0,0 +1,669 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MP2629 battery charger driver
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/types.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+#define MP2629_REG_INPUT_ILIM 0x00
+#define MP2629_REG_INPUT_VLIM 0x01
+#define MP2629_REG_CHARGE_CTRL 0x04
+#define MP2629_REG_CHARGE_ILIM 0x05
+#define MP2629_REG_PRECHARGE 0x06
+#define MP2629_REG_TERM_CURRENT 0x06
+#define MP2629_REG_CHARGE_VLIM 0x07
+#define MP2629_REG_TIMER_CTRL 0x08
+#define MP2629_REG_IMPEDANCE_COMP 0x09
+#define MP2629_REG_INTERRUPT 0x0b
+#define MP2629_REG_STATUS 0x0c
+#define MP2629_REG_FAULT 0x0d
+
+#define MP2629_MASK_INPUT_TYPE GENMASK(7, 5)
+#define MP2629_MASK_CHARGE_TYPE GENMASK(4, 3)
+#define MP2629_MASK_CHARGE_CTRL GENMASK(5, 4)
+#define MP2629_MASK_WDOG_CTRL GENMASK(5, 4)
+#define MP2629_MASK_IMPEDANCE GENMASK(7, 4)
+
+#define MP2629_INPUTSOURCE_CHANGE GENMASK(7, 5)
+#define MP2629_CHARGING_CHANGE GENMASK(4, 3)
+#define MP2629_FAULT_BATTERY BIT(3)
+#define MP2629_FAULT_THERMAL BIT(4)
+#define MP2629_FAULT_INPUT BIT(5)
+#define MP2629_FAULT_OTG BIT(6)
+
+#define MP2629_MAX_BATT_CAPACITY 100
+
+#define MP2629_PROPS(_idx, _min, _max, _step) \
+ [_idx] = { \
+ .min = _min, \
+ .max = _max, \
+ .step = _step, \
+}
+
+enum mp2629_source_type {
+ MP2629_SOURCE_TYPE_NO_INPUT,
+ MP2629_SOURCE_TYPE_NON_STD,
+ MP2629_SOURCE_TYPE_SDP,
+ MP2629_SOURCE_TYPE_CDP,
+ MP2629_SOURCE_TYPE_DCP,
+ MP2629_SOURCE_TYPE_OTG = 7,
+};
+
+enum mp2629_field {
+ INPUT_ILIM,
+ INPUT_VLIM,
+ CHARGE_ILIM,
+ CHARGE_VLIM,
+ PRECHARGE,
+ TERM_CURRENT,
+ MP2629_MAX_FIELD
+};
+
+struct mp2629_charger {
+ struct device *dev;
+ int status;
+ int fault;
+
+ struct regmap *regmap;
+ struct regmap_field *regmap_fields[MP2629_MAX_FIELD];
+ struct mutex lock;
+ struct power_supply *usb;
+ struct power_supply *battery;
+ struct iio_channel *iiochan[MP2629_ADC_CHAN_END];
+};
+
+struct mp2629_prop {
+ int reg;
+ int mask;
+ int min;
+ int max;
+ int step;
+ int shift;
+};
+
+static enum power_supply_usb_type mp2629_usb_types[] = {
+ POWER_SUPPLY_USB_TYPE_SDP,
+ POWER_SUPPLY_USB_TYPE_DCP,
+ POWER_SUPPLY_USB_TYPE_CDP,
+ POWER_SUPPLY_USB_TYPE_PD_DRP,
+ POWER_SUPPLY_USB_TYPE_UNKNOWN
+};
+
+static enum power_supply_property mp2629_charger_usb_props[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_USB_TYPE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT,
+};
+
+static enum power_supply_property mp2629_charger_bat_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
+ POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+};
+
+static struct mp2629_prop props[] = {
+ MP2629_PROPS(INPUT_ILIM, 100000, 3250000, 50000),
+ MP2629_PROPS(INPUT_VLIM, 3800000, 5300000, 100000),
+ MP2629_PROPS(CHARGE_ILIM, 320000, 4520000, 40000),
+ MP2629_PROPS(CHARGE_VLIM, 3400000, 4670000, 10000),
+ MP2629_PROPS(PRECHARGE, 120000, 720000, 40000),
+ MP2629_PROPS(TERM_CURRENT, 80000, 680000, 40000),
+};
+
+static const struct reg_field mp2629_reg_fields[] = {
+ [INPUT_ILIM] = REG_FIELD(MP2629_REG_INPUT_ILIM, 0, 5),
+ [INPUT_VLIM] = REG_FIELD(MP2629_REG_INPUT_VLIM, 0, 3),
+ [CHARGE_ILIM] = REG_FIELD(MP2629_REG_CHARGE_ILIM, 0, 6),
+ [CHARGE_VLIM] = REG_FIELD(MP2629_REG_CHARGE_VLIM, 1, 7),
+ [PRECHARGE] = REG_FIELD(MP2629_REG_PRECHARGE, 4, 7),
+ [TERM_CURRENT] = REG_FIELD(MP2629_REG_TERM_CURRENT, 0, 3),
+};
+
+static char *adc_chan_name[] = {
+ "mp2629-batt-volt",
+ "mp2629-system-volt",
+ "mp2629-input-volt",
+ "mp2629-batt-current",
+ "mp2629-input-current",
+};
+
+static int mp2629_read_adc(struct mp2629_charger *charger,
+ enum mp2629_adc_chan ch,
+ union power_supply_propval *val)
+{
+ int ret;
+ int chval;
+
+ ret = iio_read_channel_processed(charger->iiochan[ch], &chval);
+ if (ret)
+ return ret;
+
+ val->intval = chval * 1000;
+
+ return 0;
+}
+
+static int mp2629_get_prop(struct mp2629_charger *charger,
+ enum mp2629_field fld,
+ union power_supply_propval *val)
+{
+ int ret;
+ unsigned int rval;
+
+ ret = regmap_field_read(charger->regmap_fields[fld], &rval);
+ if (ret)
+ return ret;
+
+ val->intval = rval * props[fld].step + props[fld].min;
+
+ return 0;
+}
+
+static int mp2629_set_prop(struct mp2629_charger *charger,
+ enum mp2629_field fld,
+ const union power_supply_propval *val)
+{
+ unsigned int rval;
+
+ if (val->intval < props[fld].min || val->intval > props[fld].max)
+ return -EINVAL;
+
+ rval = (val->intval - props[fld].min) / props[fld].step;
+ return regmap_field_write(charger->regmap_fields[fld], rval);
+}
+
+static int mp2629_get_battery_capacity(struct mp2629_charger *charger,
+ union power_supply_propval *val)
+{
+ union power_supply_propval vnow, vlim;
+ int ret;
+
+ ret = mp2629_read_adc(charger, MP2629_BATT_VOLT, &vnow);
+ if (ret)
+ return ret;
+
+ ret = mp2629_get_prop(charger, CHARGE_VLIM, &vlim);
+ if (ret)
+ return ret;
+
+ val->intval = (vnow.intval * 100) / vlim.intval;
+ val->intval = min(val->intval, MP2629_MAX_BATT_CAPACITY);
+
+ return 0;
+}
+
+static int mp2629_charger_battery_get_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+ unsigned int rval;
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = mp2629_read_adc(charger, MP2629_BATT_VOLT, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = mp2629_read_adc(charger, MP2629_BATT_CURRENT, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+ val->intval = 4520000;
+ break;
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+ val->intval = 4670000;
+ break;
+
+ case POWER_SUPPLY_PROP_CAPACITY:
+ ret = mp2629_get_battery_capacity(charger, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
+ ret = mp2629_get_prop(charger, TERM_CURRENT, val);
+ break;
+
+ case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+ ret = mp2629_get_prop(charger, PRECHARGE, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ ret = mp2629_get_prop(charger, CHARGE_VLIM, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+ ret = mp2629_get_prop(charger, CHARGE_ILIM, val);
+ break;
+
+ case POWER_SUPPLY_PROP_HEALTH:
+ if (!charger->fault)
+ val->intval = POWER_SUPPLY_HEALTH_GOOD;
+ if (MP2629_FAULT_BATTERY & charger->fault)
+ val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ else if (MP2629_FAULT_THERMAL & charger->fault)
+ val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+ else if (MP2629_FAULT_INPUT & charger->fault)
+ val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ break;
+
+ case POWER_SUPPLY_PROP_STATUS:
+ ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+ if (ret)
+ break;
+
+ rval = (rval & MP2629_MASK_CHARGE_TYPE) >> 3;
+ switch (rval) {
+ case 0x00:
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ case 0x01:
+ case 0x10:
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case 0x11:
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ }
+ break;
+
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+ if (ret)
+ break;
+
+ rval = (rval & MP2629_MASK_CHARGE_TYPE) >> 3;
+ switch (rval) {
+ case 0x00:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ case 0x01:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ case 0x10:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+ break;
+ default:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+ }
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int mp2629_charger_battery_set_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT:
+ return mp2629_set_prop(charger, TERM_CURRENT, val);
+
+ case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
+ return mp2629_set_prop(charger, PRECHARGE, val);
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+ return mp2629_set_prop(charger, CHARGE_VLIM, val);
+
+ case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+ return mp2629_set_prop(charger, CHARGE_ILIM, val);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mp2629_charger_usb_get_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+ unsigned int rval;
+ int ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+ if (ret)
+ break;
+
+ val->intval = !!(rval & MP2629_MASK_INPUT_TYPE);
+ break;
+
+ case POWER_SUPPLY_PROP_USB_TYPE:
+ ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+ if (ret)
+ break;
+
+ rval = (rval & MP2629_MASK_INPUT_TYPE) >> 5;
+ switch (rval) {
+ case MP2629_SOURCE_TYPE_SDP:
+ val->intval = POWER_SUPPLY_USB_TYPE_SDP;
+ break;
+ case MP2629_SOURCE_TYPE_CDP:
+ val->intval = POWER_SUPPLY_USB_TYPE_CDP;
+ break;
+ case MP2629_SOURCE_TYPE_DCP:
+ val->intval = POWER_SUPPLY_USB_TYPE_DCP;
+ break;
+ case MP2629_SOURCE_TYPE_OTG:
+ val->intval = POWER_SUPPLY_USB_TYPE_PD_DRP;
+ break;
+ default:
+ val->intval = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+ break;
+ }
+ break;
+
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ ret = mp2629_read_adc(charger, MP2629_INPUT_VOLT, val);
+ break;
+
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ ret = mp2629_read_adc(charger, MP2629_INPUT_CURRENT, val);
+ break;
+
+ case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+ ret = mp2629_get_prop(charger, INPUT_VLIM, val);
+ break;
+
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ ret = mp2629_get_prop(charger, INPUT_ILIM, val);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int mp2629_charger_usb_set_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(psy->dev.parent);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT:
+ return mp2629_set_prop(charger, INPUT_VLIM, val);
+
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return mp2629_set_prop(charger, INPUT_ILIM, val);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int mp2629_charger_battery_prop_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return (psp == POWER_SUPPLY_PROP_PRECHARGE_CURRENT) ||
+ (psp == POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT) ||
+ (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT) ||
+ (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE);
+}
+
+static int mp2629_charger_usb_prop_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return (psp == POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT) ||
+ (psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT);
+}
+
+static irqreturn_t mp2629_irq_handler(int irq, void *dev_id)
+{
+ struct mp2629_charger *charger = dev_id;
+ unsigned int rval;
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ ret = regmap_read(charger->regmap, MP2629_REG_FAULT, &rval);
+ if (ret)
+ goto unlock;
+
+ if (rval) {
+ charger->fault = rval;
+ if (MP2629_FAULT_BATTERY & rval)
+ dev_err(charger->dev, "Battery fault OVP\n");
+ else if (MP2629_FAULT_THERMAL & rval)
+ dev_err(charger->dev, "Thermal shutdown fault\n");
+ else if (MP2629_FAULT_INPUT & rval)
+ dev_err(charger->dev, "no input or input OVP\n");
+ else if (MP2629_FAULT_OTG & rval)
+ dev_err(charger->dev, "VIN overloaded\n");
+
+ goto unlock;
+ }
+
+ ret = regmap_read(charger->regmap, MP2629_REG_STATUS, &rval);
+ if (ret)
+ goto unlock;
+
+ if (rval & MP2629_INPUTSOURCE_CHANGE)
+ power_supply_changed(charger->usb);
+ else if (rval & MP2629_CHARGING_CHANGE)
+ power_supply_changed(charger->battery);
+
+unlock:
+ mutex_unlock(&charger->lock);
+
+ return IRQ_HANDLED;
+}
+
+static const struct power_supply_desc mp2629_usb_desc = {
+ .name = "mp2629_usb",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .usb_types = mp2629_usb_types,
+ .num_usb_types = ARRAY_SIZE(mp2629_usb_types),
+ .properties = mp2629_charger_usb_props,
+ .num_properties = ARRAY_SIZE(mp2629_charger_usb_props),
+ .get_property = mp2629_charger_usb_get_prop,
+ .set_property = mp2629_charger_usb_set_prop,
+ .property_is_writeable = mp2629_charger_usb_prop_writeable,
+};
+
+static const struct power_supply_desc mp2629_battery_desc = {
+ .name = "mp2629_battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = mp2629_charger_bat_props,
+ .num_properties = ARRAY_SIZE(mp2629_charger_bat_props),
+ .get_property = mp2629_charger_battery_get_prop,
+ .set_property = mp2629_charger_battery_set_prop,
+ .property_is_writeable = mp2629_charger_battery_prop_writeable,
+};
+
+static ssize_t batt_impedance_compensation_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(dev->parent);
+ unsigned int rval;
+ int ret;
+
+ ret = regmap_read(charger->regmap, MP2629_REG_IMPEDANCE_COMP, &rval);
+ if (ret)
+ return ret;
+
+ rval = (rval >> 4) * 10;
+ return sprintf(buf, "%d mohm\n", rval);
+}
+
+static ssize_t batt_impedance_compensation_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ struct mp2629_charger *charger = dev_get_drvdata(dev->parent);
+ unsigned int val;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 140)
+ return -ERANGE;
+
+ /* multiples of 10 mohm so round off */
+ val = val / 10;
+ ret = regmap_update_bits(charger->regmap, MP2629_REG_IMPEDANCE_COMP,
+ MP2629_MASK_IMPEDANCE, val << 4);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(batt_impedance_compensation);
+
+static struct attribute *mp2629_charger_sysfs_attrs[] = {
+ &dev_attr_batt_impedance_compensation.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(mp2629_charger_sysfs);
+
+static void mp2629_charger_disable(void *data)
+{
+ struct mp2629_charger *charger = data;
+
+ regmap_update_bits(charger->regmap, MP2629_REG_CHARGE_CTRL,
+ MP2629_MASK_CHARGE_CTRL, 0);
+}
+
+static int mp2629_charger_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mp2629_data *ddata = dev_get_drvdata(dev->parent);
+ struct mp2629_charger *charger;
+ struct power_supply_config psy_cfg = {};
+ int ret, i, irq;
+
+ charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ charger->regmap = ddata->regmap;
+ charger->dev = dev;
+ platform_set_drvdata(pdev, charger);
+
+ irq = platform_get_irq_optional(to_platform_device(dev->parent), 0);
+ if (irq < 0) {
+ dev_err(dev, "get irq fail: %d\n", irq);
+ return irq;
+ }
+
+ for (i = 0; i < MP2629_MAX_FIELD; i++) {
+ charger->regmap_fields[i] = devm_regmap_field_alloc(dev,
+ charger->regmap, mp2629_reg_fields[i]);
+ if (IS_ERR(charger->regmap_fields[i])) {
+ dev_err(dev, "regmap field alloc fail %d\n", i);
+ return PTR_ERR(charger->regmap_fields[i]);
+ }
+ }
+
+ for (i = 0; i < MP2629_ADC_CHAN_END; i++) {
+ charger->iiochan[i] = devm_iio_channel_get(dev,
+ adc_chan_name[i]);
+ if (IS_ERR(charger->iiochan[i])) {
+ dev_err(dev, "iio chan get %s err\n", adc_chan_name[i]);
+ return PTR_ERR(charger->iiochan[i]);
+ }
+ }
+
+ ret = devm_add_action_or_reset(dev, mp2629_charger_disable, charger);
+ if (ret)
+ return ret;
+
+ charger->usb = devm_power_supply_register(dev, &mp2629_usb_desc, NULL);
+ if (IS_ERR(charger->usb)) {
+ dev_err(dev, "power supply register usb failed\n");
+ return PTR_ERR(charger->usb);
+ }
+
+ psy_cfg.drv_data = charger;
+ psy_cfg.attr_grp = mp2629_charger_sysfs_groups;
+ charger->battery = devm_power_supply_register(dev,
+ &mp2629_battery_desc, &psy_cfg);
+ if (IS_ERR(charger->battery)) {
+ dev_err(dev, "power supply register battery failed\n");
+ return PTR_ERR(charger->battery);
+ }
+
+ ret = regmap_update_bits(charger->regmap, MP2629_REG_CHARGE_CTRL,
+ MP2629_MASK_CHARGE_CTRL, BIT(4));
+ if (ret) {
+ dev_err(dev, "enable charge fail: %d\n", ret);
+ return ret;
+ }
+
+ regmap_update_bits(charger->regmap, MP2629_REG_TIMER_CTRL,
+ MP2629_MASK_WDOG_CTRL, 0);
+
+ mutex_init(&charger->lock);
+
+ ret = devm_request_threaded_irq(dev, irq, NULL, mp2629_irq_handler,
+ IRQF_ONESHOT | IRQF_TRIGGER_RISING,
+ "mp2629-charger", charger);
+ if (ret) {
+ dev_err(dev, "failed to request gpio IRQ\n");
+ return ret;
+ }
+
+ regmap_update_bits(charger->regmap, MP2629_REG_INTERRUPT,
+ GENMASK(6, 5), BIT(6) | BIT(5));
+
+ return 0;
+}
+
+static const struct of_device_id mp2629_charger_of_match[] = {
+ { .compatible = "mps,mp2629_charger"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, mp2629_charger_of_match);
+
+static struct platform_driver mp2629_charger_driver = {
+ .driver = {
+ .name = "mp2629_charger",
+ .of_match_table = mp2629_charger_of_match,
+ },
+ .probe = mp2629_charger_probe,
+};
+module_platform_driver(mp2629_charger_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 Charger driver");
+MODULE_LICENSE("GPL");
--
2.17.1
^ permalink raw reply related
* [PATCH v12 3/6] iio: adc: mp2629: Add support for mp2629 ADC driver
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
In-Reply-To: <20200526090646.25827-1-sravanhome@gmail.com>
Add support for 8-bit resolution ADC readings for input power
supply and battery charging measurement. Provides voltage, current
readings to mp2629 power supply driver.
Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mp2629_adc.c | 208 +++++++++++++++++++++++++++++++++++
include/linux/mfd/mp2629.h | 9 ++
4 files changed, 228 insertions(+)
create mode 100644 drivers/iio/adc/mp2629_adc.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 82e33082958c..ef0c0cd31855 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -680,6 +680,16 @@ config MESON_SARADC
To compile this driver as a module, choose M here: the
module will be called meson_saradc.
+config MP2629_ADC
+ tristate "Monolithic MP2629 ADC driver"
+ depends on MFD_MP2629
+ help
+ Say yes to have support for battery charger IC MP2629 ADC device
+ accessed over I2C.
+
+ This driver provides ADC conversion of system, input power supply
+ and battery voltage & current information.
+
config NAU7802
tristate "Nuvoton NAU7802 ADC driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 919228900df9..f14416c245a6 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
+obj-$(CONFIG_MP2629_ADC) += mp2629_adc.o
obj-$(CONFIG_MXS_LRADC_ADC) += mxs-lradc-adc.o
obj-$(CONFIG_NAU7802) += nau7802.o
obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
diff --git a/drivers/iio/adc/mp2629_adc.c b/drivers/iio/adc/mp2629_adc.c
new file mode 100644
index 000000000000..331a9a728217
--- /dev/null
+++ b/drivers/iio/adc/mp2629_adc.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * MP2629 Driver for ADC
+ *
+ * Copyright 2020 Monolithic Power Systems, Inc
+ *
+ * Author: Saravanan Sekar <sravanhome@gmail.com>
+ */
+
+#include <linux/iio/driver.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/mfd/mp2629.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MP2629_REG_ADC_CTRL 0x03
+#define MP2629_REG_BATT_VOLT 0x0e
+#define MP2629_REG_SYSTEM_VOLT 0x0f
+#define MP2629_REG_INPUT_VOLT 0x11
+#define MP2629_REG_BATT_CURRENT 0x12
+#define MP2629_REG_INPUT_CURRENT 0x13
+
+#define MP2629_ADC_START BIT(7)
+#define MP2629_ADC_CONTINUOUS BIT(6)
+
+#define MP2629_MAP(_mp, _mpc) IIO_MAP(#_mp, "mp2629_charger", "mp2629-"_mpc)
+
+#define MP2629_ADC_CHAN(_ch, _type) { \
+ .type = _type, \
+ .indexed = 1, \
+ .datasheet_name = #_ch, \
+ .channel = MP2629_ ## _ch, \
+ .address = MP2629_REG_ ## _ch, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+struct mp2629_adc {
+ struct regmap *regmap;
+ struct device *dev;
+};
+
+static struct iio_chan_spec mp2629_channels[] = {
+ MP2629_ADC_CHAN(BATT_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(SYSTEM_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(INPUT_VOLT, IIO_VOLTAGE),
+ MP2629_ADC_CHAN(BATT_CURRENT, IIO_CURRENT),
+ MP2629_ADC_CHAN(INPUT_CURRENT, IIO_CURRENT)
+};
+
+static struct iio_map mp2629_adc_maps[] = {
+ MP2629_MAP(BATT_VOLT, "batt-volt"),
+ MP2629_MAP(SYSTEM_VOLT, "system-volt"),
+ MP2629_MAP(INPUT_VOLT, "input-volt"),
+ MP2629_MAP(BATT_CURRENT, "batt-current"),
+ MP2629_MAP(INPUT_CURRENT, "input-current")
+};
+
+static int mp2629_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct mp2629_adc *info = iio_priv(indio_dev);
+ unsigned int rval;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = regmap_read(info->regmap, chan->address, &rval);
+ if (ret)
+ return ret;
+
+ if (chan->address == MP2629_INPUT_VOLT)
+ rval &= GENMASK(6, 0);
+ *val = rval;
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->channel) {
+ case MP2629_BATT_VOLT:
+ case MP2629_SYSTEM_VOLT:
+ *val = 20;
+ return IIO_VAL_INT;
+
+ case MP2629_INPUT_VOLT:
+ *val = 60;
+ return IIO_VAL_INT;
+
+ case MP2629_BATT_CURRENT:
+ *val = 175;
+ *val2 = 10;
+ return IIO_VAL_FRACTIONAL;
+
+ case MP2629_INPUT_CURRENT:
+ *val = 133;
+ *val2 = 10;
+ return IIO_VAL_FRACTIONAL;
+
+ default:
+ return -EINVAL;
+ }
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info mp2629_adc_info = {
+ .read_raw = &mp2629_read_raw,
+};
+
+static int mp2629_adc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mp2629_data *ddata = dev_get_drvdata(dev->parent);
+ struct mp2629_adc *info;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ info = iio_priv(indio_dev);
+ info->regmap = ddata->regmap;
+ info->dev = dev;
+ platform_set_drvdata(pdev, indio_dev);
+
+ ret = regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START | MP2629_ADC_CONTINUOUS,
+ MP2629_ADC_START | MP2629_ADC_CONTINUOUS);
+ if (ret) {
+ dev_err(dev, "adc enable fail: %d\n", ret);
+ return ret;
+ }
+
+ ret = iio_map_array_register(indio_dev, mp2629_adc_maps);
+ if (ret) {
+ dev_err(dev, "IIO maps register fail: %d\n", ret);
+ goto fail_disable;
+ }
+
+ indio_dev->name = "mp2629-adc";
+ indio_dev->dev.parent = dev;
+ indio_dev->channels = mp2629_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mp2629_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &mp2629_adc_info;
+
+ ret = iio_device_register(indio_dev);
+ if (ret) {
+ dev_err(dev, "IIO device register fail: %d\n", ret);
+ goto fail_map_unregister;
+ }
+
+ return 0;
+
+fail_map_unregister:
+ iio_map_array_unregister(indio_dev);
+
+fail_disable:
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_CONTINUOUS, 0);
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START, 0);
+
+ return ret;
+}
+
+static int mp2629_adc_remove(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct mp2629_adc *info = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+
+ iio_map_array_unregister(indio_dev);
+
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_CONTINUOUS, 0);
+ regmap_update_bits(info->regmap, MP2629_REG_ADC_CTRL,
+ MP2629_ADC_START, 0);
+
+ return 0;
+}
+
+static const struct of_device_id mp2629_adc_of_match[] = {
+ { .compatible = "mps,mp2629_adc"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, mp2629_adc_of_match);
+
+static struct platform_driver mp2629_adc_driver = {
+ .driver = {
+ .name = "mp2629_adc",
+ .of_match_table = mp2629_adc_of_match,
+ },
+ .probe = mp2629_adc_probe,
+ .remove = mp2629_adc_remove,
+};
+module_platform_driver(mp2629_adc_driver);
+
+MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>");
+MODULE_DESCRIPTION("MP2629 ADC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h
index baaeeaf82949..89b706900b57 100644
--- a/include/linux/mfd/mp2629.h
+++ b/include/linux/mfd/mp2629.h
@@ -14,4 +14,13 @@ struct mp2629_data {
struct regmap *regmap;
};
+enum mp2629_adc_chan {
+ MP2629_BATT_VOLT,
+ MP2629_SYSTEM_VOLT,
+ MP2629_INPUT_VOLT,
+ MP2629_BATT_CURRENT,
+ MP2629_INPUT_CURRENT,
+ MP2629_ADC_CHAN_END
+};
+
#endif
--
2.17.1
^ permalink raw reply related
* [PATCH v12 0/6] Add battery charger driver support for MP2629
From: Saravanan Sekar @ 2020-05-26 9:06 UTC (permalink / raw)
To: lee.jones, andy.shevchenko, robh+dt, jic23, knaack.h, lars,
pmeerw, sre
Cc: devicetree, linux-kernel, linux-iio, linux-pm, sravanhome
changes in v12:
- added SOB Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
changes in v11:
- module_remove part replaced by devm_add_action_or_reset in charger patch
- minor review comments solved in mfd and adc patch
I expect, that Lee will provide an immutable branch for me and
Jonathan once the MFD bits have been reviewed. Please tell me,
if I should prepare one instead.
-- Sebastian
changes in v10:
- fixed typo, \n in dev_err
- dt bindings Warning (unit_address_vs_reg) reported by bot
changes in v9:
- fixed review comments in mp2629 power supply such as resource based
iio channel, replace workqueue by threaded irq, irq get with "_optional"
changes in v8:
- fixed order of call in probe/remove in iio adc
- add ABI documentation for mp2629 power supply
changes in v7:
- fixed probe/remove order, managed and unmanaged call mix use in adc.
- Documentation dual license, i2c node with controller address
Overall looks good to me, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
One question though in reply to patch 4.
changes in v6:
- removed includes types.h in mfd, of_device.h in adc.
- fixed review comments parentheses, err check, kstrtouint
changes in v5:
- removed platfrom data stored in mfd and directly accessed mfd struct in child
- fixed spell check and capitalization in mfd and documentation
changes in v4:
- fixed capitalization in mfg Kconfig and documentation
changes in v3:
- regmap for children passed using platform data and remove mfd driver info
access directly from children
changes in v2:
- removed EXPORT_SYMBOL of register set/get helper
- regmap bit filed used, fixed other review comments
This patch series add support for Battery charger control driver for Monolithic
Power System's MP2629 chipset, includes MFD driver for ADC battery & input
power supply measurement and battery charger control driver.
Thanks,
Saravanan
Saravanan Sekar (6):
dt-bindings: mfd: add document bindings for mp2629
mfd: mp2629: Add support for mps battery charger
iio: adc: mp2629: Add support for mp2629 ADC driver
power: supply: Add support for mps mp2629 battery charger
power: supply: mp2629: Add impedance compensation config
MAINTAINERS: Add entry for mp2629 Battery Charger driver
.../ABI/testing/sysfs-class-power-mp2629 | 8 +
.../devicetree/bindings/mfd/mps,mp2629.yaml | 62 ++
MAINTAINERS | 5 +
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mp2629_adc.c | 208 ++++++
drivers/mfd/Kconfig | 9 +
drivers/mfd/Makefile | 2 +
drivers/mfd/mp2629.c | 79 +++
drivers/power/supply/Kconfig | 10 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/mp2629_charger.c | 669 ++++++++++++++++++
include/linux/mfd/mp2629.h | 26 +
13 files changed, 1090 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-class-power-mp2629
create mode 100644 Documentation/devicetree/bindings/mfd/mps,mp2629.yaml
create mode 100644 drivers/iio/adc/mp2629_adc.c
create mode 100644 drivers/mfd/mp2629.c
create mode 100644 drivers/power/supply/mp2629_charger.c
create mode 100644 include/linux/mfd/mp2629.h
--
2.17.1
^ permalink raw reply
* RE: [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove module support
From: Sandipan Patra @ 2020-05-26 9:05 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Thierry Reding, Jonathan Hunter, kamil@wypas.org,
jdelvare@suse.com, linux@roeck-us.net, robh+dt@kernel.org,
Bibek Basu, Bitan Biswas, linux-pwm@vger.kernel.org,
linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200526070450.sackolk6m3qv7omy@pengutronix.de>
Hi Uwe,
> -----Original Message-----
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Sent: Tuesday, May 26, 2020 12:35 PM
> To: Sandipan Patra <spatra@nvidia.com>
> Cc: Thierry Reding <treding@nvidia.com>; Jonathan Hunter
> <jonathanh@nvidia.com>; kamil@wypas.org; jdelvare@suse.com; linux@roeck-
> us.net; robh+dt@kernel.org; Bibek Basu <bbasu@nvidia.com>; Bitan Biswas
> <bbiswas@nvidia.com>; linux-pwm@vger.kernel.org; linux-
> hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux-
> tegra@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove
> module support
>
> External email: Use caution opening links or attachments
>
>
> On Tue, May 26, 2020 at 10:36:04AM +0530, Sandipan Patra wrote:
> > This change has 2 parts:
> > 1. Add support for profiles mode settings.
> > This allows different fan settings for trip point temp/hyst/pwm.
> > T194 has multiple fan-profiles support.
> >
> > 2. Add pwm-fan remove support. This is essential since the config is
> > tristate capable.
>
> These two are orthogonal, aren't they? So they belong in two patches.
>
> You have to expand the binding documentation.
>
> > Signed-off-by: Sandipan Patra <spatra@nvidia.com>
> > ---
> > drivers/hwmon/pwm-fan.c | 112
> > ++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 100 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c index
> > 30b7b3e..26db589 100644
> > --- a/drivers/hwmon/pwm-fan.c
> > +++ b/drivers/hwmon/pwm-fan.c
> > @@ -3,8 +3,10 @@
> > * pwm-fan.c - Hwmon driver for fans connected to PWM lines.
> > *
> > * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> > + * Copyright (c) 2020, NVIDIA Corporation.
> > *
> > * Author: Kamil Debski <k.debski@samsung.com>
> > + * Author: Sandipan Patra <spatra@nvidia.com>
> > */
> >
> > #include <linux/hwmon.h>
> > @@ -21,6 +23,8 @@
> > #include <linux/timer.h>
> >
> > #define MAX_PWM 255
> > +/* Based on OF max device tree node name length */
> > +#define MAX_PROFILE_NAME_LENGTH 31
> >
> > struct pwm_fan_ctx {
> > struct mutex lock;
> > @@ -38,6 +42,12 @@ struct pwm_fan_ctx {
> > unsigned int pwm_fan_state;
> > unsigned int pwm_fan_max_state;
> > unsigned int *pwm_fan_cooling_levels;
> > +
> > + unsigned int pwm_fan_profiles;
> > + const char **fan_profile_names;
> > + unsigned int **fan_profile_cooling_levels;
> > + unsigned int fan_current_profile;
> > +
> > struct thermal_cooling_device *cdev; };
> >
> > @@ -227,28 +237,86 @@ static int pwm_fan_of_get_cooling_data(struct
> device *dev,
> > struct pwm_fan_ctx *ctx) {
> > struct device_node *np = dev->of_node;
> > + struct device_node *base_profile = NULL;
> > + struct device_node *profile_np = NULL;
> > + const char *default_profile = NULL;
> > int num, i, ret;
> >
> > - if (!of_find_property(np, "cooling-levels", NULL))
> > - return 0;
> > + num = of_property_count_u32_elems(np, "cooling-levels");
> > + if (num <= 0) {
> > + base_profile = of_get_child_by_name(np, "profiles");
> > + if (!base_profile) {
> > + dev_err(dev, "Wrong Data\n");
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (base_profile) {
> > + ctx->pwm_fan_profiles =
> > + of_get_available_child_count(base_profile);
> > +
> > + if (ctx->pwm_fan_profiles <= 0) {
> > + dev_err(dev, "Profiles used but not defined\n");
> > + return -EINVAL;
> > + }
> >
> > - ret = of_property_count_u32_elems(np, "cooling-levels");
> > - if (ret <= 0) {
> > - dev_err(dev, "Wrong data!\n");
> > - return ret ? : -EINVAL;
> > + ctx->fan_profile_names = devm_kzalloc(dev,
> > + sizeof(const char *) * ctx->pwm_fan_profiles,
> > + GFP_KERNEL);
> > + ctx->fan_profile_cooling_levels = devm_kzalloc(dev,
> > + sizeof(int *) * ctx->pwm_fan_profiles,
> > + GFP_KERNEL);
> > +
> > + if (!ctx->fan_profile_names
> > + || !ctx->fan_profile_cooling_levels)
> > + return -ENOMEM;
> > +
> > + ctx->fan_current_profile = 0;
> > + i = 0;
> > + for_each_available_child_of_node(base_profile, profile_np) {
> > + num = of_property_count_u32_elems(profile_np,
> > + "cooling-levels");
> > + if (num <= 0) {
> > + dev_err(dev, "No data in cooling-levels inside profile
> node!\n");
> > + return -EINVAL;
> > + }
> > +
> > + of_property_read_string(profile_np, "name",
> > + &ctx->fan_profile_names[i]);
> > + if (default_profile &&
> > + !strncmp(default_profile,
> > + ctx->fan_profile_names[i],
> > + MAX_PROFILE_NAME_LENGTH))
> > + ctx->fan_current_profile = i;
> > +
> > + ctx->fan_profile_cooling_levels[i] =
> > + devm_kzalloc(dev, sizeof(int) * num,
> > + GFP_KERNEL);
> > + if (!ctx->fan_profile_cooling_levels[i])
> > + return -ENOMEM;
> > +
> > + of_property_read_u32_array(profile_np, "cooling-levels",
> > + ctx->fan_profile_cooling_levels[i], num);
> > + i++;
> > + }
> > }
> >
> > - num = ret;
> > ctx->pwm_fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
> > GFP_KERNEL);
> > if (!ctx->pwm_fan_cooling_levels)
> > return -ENOMEM;
> >
> > - ret = of_property_read_u32_array(np, "cooling-levels",
> > - ctx->pwm_fan_cooling_levels, num);
> > - if (ret) {
> > - dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> > - return ret;
> > + if (base_profile) {
> > + memcpy(ctx->pwm_fan_cooling_levels,
> > + ctx->fan_profile_cooling_levels[ctx->fan_current_profile],
> > + num);
> > + } else {
> > + ret = of_property_read_u32_array(np, "cooling-levels",
> > + ctx->pwm_fan_cooling_levels, num);
> > + if (ret) {
> > + dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> > + return -EINVAL;
> > + }
> > }
> >
> > for (i = 0; i < num; i++) {
> > @@ -390,6 +458,25 @@ static int pwm_fan_probe(struct platform_device
> *pdev)
> > return 0;
> > }
> >
> > +static int pwm_fan_remove(struct platform_device *pdev) {
> > + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
> > + struct pwm_args args;
> > +
> > + if (!ctx)
> > + return -EINVAL;
> > +
> > + if (IS_ENABLED(CONFIG_THERMAL))
> > + thermal_cooling_device_unregister(ctx->cdev);
> > +
> > + pwm_get_args(ctx->pwm, &args);
> > + pwm_config(ctx->pwm, 0, args.period);
> > + pwm_disable(ctx->pwm);
>
> What is what you really here? Is it only that the PWM stops oscillating, or is it
> crucial that the output goes to its inactive level?
>
> (The intended semantic of pwm_disable includes that the output goes low, but
> not all implementations enforce this.)
>
> Also please don't introduce new users of pwm_config() and pwm_disable() use
> pwm_apply() instead.
>
> I wonder if this unregistration is "safe". When the driver is in use I'd expect that
> the hwmon device doesn't go away and so the devm unregistration callback that
> belongs to
> devm_hwmon_device_register_with_groups() blocks. But at this time the PWM
> is already stopped and so the device stops functioning earlier.
>
> Best regards
> Uwe
>
Thanks for reviewing the changes.
I see that pwm_fan_shutdown() which has got merged recently, can also be used for
module remove functionality. May be it will need a little bit of tweak in the code.
However I should have not made both multiple profiles support and fan remove functionality on
same patch.
For now I will prepare the current patch only to handle multiple profiles and corresponding
device tree settings.
Thanks & Regards,
Sandipan
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v3 04/10] media: i2c: imx290: Add support for 2 data lanes
From: Sakari Ailus @ 2020-05-26 9:01 UTC (permalink / raw)
To: Andrey Konovalov
Cc: mchehab, manivannan.sadhasivam, linux-media, linux-kernel,
devicetree, linux-arm-kernel, c.barrett, a.brela, peter.griffin
In-Reply-To: <20200524192505.20682-5-andrey.konovalov@linaro.org>
Hi Andrey,
On Sun, May 24, 2020 at 10:24:59PM +0300, Andrey Konovalov wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> The IMX290 sensor can output frames with 2/4 CSI2 data lanes. This commit
> adds support for 2 lane mode in addition to the 4 lane and also
> configuring the data lane settings in the driver based on system
> configuration.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> ---
> drivers/media/i2c/imx290.c | 133 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 124 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 7b1de1f0c8b7..a361c9ac8bd5 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -25,7 +25,18 @@
> #define IMX290_STANDBY 0x3000
> #define IMX290_REGHOLD 0x3001
> #define IMX290_XMSTA 0x3002
> +#define IMX290_FR_FDG_SEL 0x3009
> #define IMX290_GAIN 0x3014
> +#define IMX290_HMAX_LOW 0x301c
> +#define IMX290_HMAX_HIGH 0x301d
> +#define IMX290_PHY_LANE_NUM 0x3407
> +#define IMX290_CSI_LANE_MODE 0x3443
> +
> +/* HMAX fields */
> +#define IMX290_HMAX_2_1920 0x1130
> +#define IMX290_HMAX_4_1920 0x0898
> +#define IMX290_HMAX_2_720 0x19C8
> +#define IMX290_HMAX_4_720 0x0CE4
>
> #define IMX290_DEFAULT_LINK_FREQ 445500000
>
> @@ -56,6 +67,7 @@ struct imx290 {
> struct device *dev;
> struct clk *xclk;
> struct regmap *regmap;
> + u8 nlanes;
>
> struct v4l2_subdev sd;
> struct v4l2_fwnode_endpoint ep;
> @@ -89,14 +101,11 @@ static const struct regmap_config imx290_regmap_config = {
>
> static const struct imx290_regval imx290_global_init_settings[] = {
> { 0x3007, 0x00 },
> - { 0x3009, 0x00 },
> { 0x3018, 0x65 },
> { 0x3019, 0x04 },
> { 0x301a, 0x00 },
> - { 0x3443, 0x03 },
> { 0x3444, 0x20 },
> { 0x3445, 0x25 },
> - { 0x3407, 0x03 },
> { 0x303a, 0x0c },
> { 0x3040, 0x00 },
> { 0x3041, 0x00 },
> @@ -169,7 +178,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
> { 0x3164, 0x1a },
> { 0x3480, 0x49 },
> /* data rate settings */
> - { 0x3009, 0x01 },
> { 0x3405, 0x10 },
> { 0x3446, 0x57 },
> { 0x3447, 0x00 },
> @@ -187,8 +195,6 @@ static const struct imx290_regval imx290_1080p_settings[] = {
> { 0x3453, 0x00 },
> { 0x3454, 0x17 },
> { 0x3455, 0x00 },
> - { 0x301c, 0x98 },
> - { 0x301d, 0x08 },
> };
>
> static const struct imx290_regval imx290_720p_settings[] = {
> @@ -210,7 +216,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
> { 0x3164, 0x1a },
> { 0x3480, 0x49 },
> /* data rate settings */
> - { 0x3009, 0x01 },
> { 0x3405, 0x10 },
> { 0x3446, 0x4f },
> { 0x3447, 0x00 },
> @@ -228,8 +233,6 @@ static const struct imx290_regval imx290_720p_settings[] = {
> { 0x3453, 0x00 },
> { 0x3454, 0x17 },
> { 0x3455, 0x00 },
> - { 0x301c, 0xe4 },
> - { 0x301d, 0x0c },
> };
>
> static const struct imx290_regval imx290_10bit_settings[] = {
> @@ -522,6 +525,25 @@ static int imx290_write_current_format(struct imx290 *imx290,
> return 0;
> }
>
> +static int imx290_set_hmax(struct imx290 *imx290, u32 val)
> +{
> + int ret;
> +
> + ret = imx290_write_reg(imx290, IMX290_HMAX_LOW, (val & 0xff));
> + if (ret) {
> + dev_err(imx290->dev, "Error setting HMAX register\n");
> + return ret;
> + }
> +
> + ret = imx290_write_reg(imx290, IMX290_HMAX_HIGH, ((val >> 8) & 0xff));
> + if (ret) {
> + dev_err(imx290->dev, "Error setting HMAX register\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /* Start streaming */
> static int imx290_start_streaming(struct imx290 *imx290)
> {
> @@ -551,6 +573,40 @@ static int imx290_start_streaming(struct imx290 *imx290)
> return ret;
> }
>
> + switch (imx290->nlanes) {
> + case 2:
> + if (imx290->current_mode->width == 1920) {
> + ret = imx290_set_hmax(imx290, IMX290_HMAX_2_1920);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = imx290_set_hmax(imx290, IMX290_HMAX_2_720);
> + if (ret < 0)
> + return ret;
> + }
> +
> + break;
> + case 4:
> + if (imx290->current_mode->width == 1920) {
> + ret = imx290_set_hmax(imx290, IMX290_HMAX_4_1920);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = imx290_set_hmax(imx290, IMX290_HMAX_4_720);
> + if (ret < 0)
> + return ret;
I think it'd be nicer to put this where the mode definitions are, to avoid
scattering the configuration around the driver.
> + }
> +
> + break;
> + default:
> + /*
> + * We should never hit this since the data lane count is
> + * validated in probe itself
> + */
> + dev_err(imx290->dev, "Lane configuration not supported\n");
> + return -EINVAL;
> + }
> +
> /* Apply customized values from user */
> ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
> if (ret) {
> @@ -607,6 +663,49 @@ static int imx290_get_regulators(struct device *dev, struct imx290 *imx290)
> imx290->supplies);
> }
>
> +static int imx290_set_data_lanes(struct imx290 *imx290)
> +{
> + int ret = 0, laneval, frsel;
> +
> + switch (imx290->nlanes) {
> + case 2:
> + laneval = 0x01;
> + frsel = 0x02;
> + break;
> + case 4:
> + laneval = 0x03;
> + frsel = 0x01;
> + break;
> + default:
> + /*
> + * We should never hit this since the data lane count is
> + * validated in probe itself
> + */
> + dev_err(imx290->dev, "Lane configuration not supported\n");
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + ret = imx290_write_reg(imx290, IMX290_PHY_LANE_NUM, laneval);
> + if (ret) {
> + dev_err(imx290->dev, "Error setting Physical Lane number register\n");
> + goto exit;
> + }
> +
> + ret = imx290_write_reg(imx290, IMX290_CSI_LANE_MODE, laneval);
> + if (ret) {
> + dev_err(imx290->dev, "Error setting CSI Lane mode register\n");
> + goto exit;
> + }
> +
> + ret = imx290_write_reg(imx290, IMX290_FR_FDG_SEL, frsel);
> + if (ret)
> + dev_err(imx290->dev, "Error setting FR/FDG SEL register\n");
> +
> +exit:
> + return ret;
> +}
> +
> static int imx290_power_on(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> @@ -631,6 +730,9 @@ static int imx290_power_on(struct device *dev)
> gpiod_set_value_cansleep(imx290->rst_gpio, 0);
> usleep_range(30000, 31000);
>
> + /* Set data lane count */
> + imx290_set_data_lanes(imx290);
> +
> return 0;
> }
>
> @@ -703,6 +805,16 @@ static int imx290_probe(struct i2c_client *client)
> goto free_err;
> }
>
> + /* Get number of data lanes */
While at it, could you set the PHY type in the V4L2 fwnode endpoint before
parsing the data using v4l2_fwnode_endpoint_alloc_parse()?
> + imx290->nlanes = imx290->ep.bus.mipi_csi2.num_data_lanes;
> + if (imx290->nlanes != 2 && imx290->nlanes != 4) {
> + dev_err(dev, "Invalid data lanes: %d\n", imx290->nlanes);
> + ret = -EINVAL;
> + goto free_err;
> + }
> +
> + dev_dbg(dev, "Using %u data lanes\n", imx290->nlanes);
> +
> if (!imx290->ep.nr_of_link_frequencies) {
> dev_err(dev, "link-frequency property not found in DT\n");
> ret = -EINVAL;
> @@ -823,6 +935,9 @@ static int imx290_probe(struct i2c_client *client)
> goto free_entity;
> }
>
> + /* Set data lane count */
> + imx290_set_data_lanes(imx290);
> +
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
> pm_runtime_idle(dev);
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Sakari Ailus @ 2020-05-26 8:54 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Kieran Bingham, linux-renesas-soc, linux-media, devicetree,
linux-kernel, Mauro Carvalho Chehab, Kieran Bingham, Jacopo Mondi,
Niklas Söderlund, Hans Verkuil, Hyun Kwon,
Manivannan Sadhasivam, Rob Herring, Jacopo Mondi,
Laurent Pinchart, Niklas Söderlund
In-Reply-To: <20200520005020.GQ3820@pendragon.ideasonboard.com>
Hi Laurent,
On Wed, May 20, 2020 at 03:50:20AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Mon, May 18, 2020 at 12:45:18PM +0100, Kieran Bingham wrote:
> > Hi Sakari,
> >
> > There are only fairly minor comments here, fix ups will be included in a
> > v10.
> >
> > Is there anything major blocking integration?
> >
> > On 16/05/2020 22:51, Sakari Ailus wrote:
> > > On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:
> > >
> > > ...
> > >
> > >> +static int max9286_enum_mbus_code(struct v4l2_subdev *sd,
> > >> + struct v4l2_subdev_pad_config *cfg,
> > >> + struct v4l2_subdev_mbus_code_enum *code)
> > >> +{
> > >> + if (code->pad || code->index > 0)
> > >> + return -EINVAL;
> > >> +
> > >> + code->code = MEDIA_BUS_FMT_UYVY8_2X8;
> > >
> > > Why UYVY8_2X8 and not UYVY8_1X16? In general, the single sample / pixel
> > > variant of the format is generally used on the serial busses. This choice
> > > was made when serial busses were introduced.
>
> This is a bit of a tricky one. On the camera size, for the RDACM20, the
> O10635 sensor outputs UYVY8_2X8. This if fed to the MAX9271 serializer,
> which doesn't care about the data type. The MAX9271 has a 16-bit input
> bus, with 10 bits reserved for data, 2 bits dynamically configurable
> to carry H/V sync or extra data, and 4 bits dynamically configurable to
> carry GPIOs or extra data. The 16-bit words are then serialized (it's a
> bit more complicated, when using the H/V sync signals they are
> transmitted in a different way, and the MAX9271 also supports a DDR mode
> that makes the "serial link word" carry up to 30 bits). Effectively, the
> two samples of UYVY8_2X8 are serialized in a 16-bit word each.
>
> Sakari, with this information in mind, what would you recommend ?
As this pad describes a serial bus --- doesn't it? --- it should use the
same formats as other serial busses such as CSI-2. I.e. the 1X16 variant.
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH 2/4] ARM: dts: r8a7742-iwg21d-q7: Enable SDHI2 controller
From: Lad, Prabhakar @ 2020-05-26 8:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Lad Prabhakar, Magnus Damm, Rob Herring, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List
In-Reply-To: <CAMuHMdWjGX43EDnwxu0xp3U7aedWHuW5MLQo==1d5v_mV-1R9A@mail.gmail.com>
Hi Geert,
On Tue, May 26, 2020 at 8:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, May 25, 2020 at 5:23 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Enable the SDHI2 controller on iWave RZ/G1H carrier board.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> i.e. will queue in renesas-devel for v5.9.
>
Thank you for the review.
> BTW, perhaps you want to add an LED trigger for SDIO_LED, connected
> to GP5_22?
>
Sure I'll add this as a separate patch.
Cheers,
--Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* [PATCH] dt-bindings: leds: fix macro names for pca955x
From: Flavio Suligoi @ 2020-05-26 8:37 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek, Dan Murphy, Rob Herring
Cc: linux-leds, devicetree, linux-kernel, Flavio Suligoi
The documentation reports the wrong macro names
related to the pca9532 instead of the pca955x
Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
---
Documentation/devicetree/bindings/leds/leds-pca955x.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/leds-pca955x.txt b/Documentation/devicetree/bindings/leds/leds-pca955x.txt
index 7984efb767b4..7a5830f8d5ab 100644
--- a/Documentation/devicetree/bindings/leds/leds-pca955x.txt
+++ b/Documentation/devicetree/bindings/leds/leds-pca955x.txt
@@ -26,9 +26,9 @@ LED sub-node properties:
from 0 to 15 for the pca9552
from 0 to 3 for the pca9553
- type: (optional) either
- PCA9532_TYPE_NONE
- PCA9532_TYPE_LED
- PCA9532_TYPE_GPIO
+ PCA955X_TYPE_NONE
+ PCA955X_TYPE_LED
+ PCA955X_TYPE_GPIO
see dt-bindings/leds/leds-pca955x.h (default to LED)
- label : (optional)
see Documentation/devicetree/bindings/leds/common.txt
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v11 2/6] mfd: mp2629: Add support for mps battery charger
From: Lee Jones @ 2020-05-26 8:27 UTC (permalink / raw)
To: saravanan sekar
Cc: andy.shevchenko, robh+dt, jic23, knaack.h, lars, pmeerw, sre,
devicetree, linux-kernel, linux-iio, linux-pm
In-Reply-To: <75b6a2ee-2d90-17c2-4b2b-0ef6238e903e@gmail.com>
On Sat, 23 May 2020, saravanan sekar wrote:
> Hi Lee,
>
> On 19/05/20 12:37 pm, Lee Jones wrote:
> > On Thu, 30 Apr 2020, Saravanan Sekar wrote:
> >
> > > mp2629 is a highly-integrated switching-mode battery charge management
> > > device for single-cell Li-ion or Li-polymer battery.
> > >
> > > Add MFD core enables chip access for ADC driver for battery readings,
> > > and a power supply battery-charger driver
> > >
> > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > ---
> > > drivers/mfd/Kconfig | 9 +++++
> > > drivers/mfd/Makefile | 2 +
> > > drivers/mfd/mp2629.c | 79 ++++++++++++++++++++++++++++++++++++++
> > > include/linux/mfd/mp2629.h | 17 ++++++++
> > > 4 files changed, 107 insertions(+)
> > > create mode 100644 drivers/mfd/mp2629.c
> > > create mode 100644 include/linux/mfd/mp2629.h
> > For my own reference (apply this as-is to your sign-off block):
> >
> > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> >
> Do I have to resend with above sign-off in all the patch?
That would make things simpler, yes.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 4/4] ARM: dts: r8a7742-iwg21d-q7: Add RWDT support
From: Geert Uytterhoeven @ 2020-05-26 7:58 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Magnus Damm, Rob Herring, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Prabhakar
In-Reply-To: <1590420129-7531-5-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, May 25, 2020 at 5:23 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Enable RWDT and use 60 seconds as default timeout.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 3/4] ARM: dts: r8a7742-iwg21d-q7: Enable Ethernet avb
From: Geert Uytterhoeven @ 2020-05-26 7:58 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Magnus Damm, Rob Herring, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Prabhakar
In-Reply-To: <1590420129-7531-4-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, May 25, 2020 at 5:23 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Enable ethernet avb on iWave RZ/G1H carrier board.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 2/4] ARM: dts: r8a7742-iwg21d-q7: Enable SDHI2 controller
From: Geert Uytterhoeven @ 2020-05-26 7:58 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Magnus Damm, Rob Herring, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Prabhakar
In-Reply-To: <1590420129-7531-3-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hi Prabhakar,
On Mon, May 25, 2020 at 5:23 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Enable the SDHI2 controller on iWave RZ/G1H carrier board.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.
BTW, perhaps you want to add an LED trigger for SDIO_LED, connected
to GP5_22?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/4] ARM: dts: r8a7742-iwg21d-q7: Enable scifb2 node
From: Geert Uytterhoeven @ 2020-05-26 7:56 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Magnus Damm, Rob Herring, Linux-Renesas,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, Prabhakar
In-Reply-To: <1590420129-7531-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, May 25, 2020 at 5:23 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Enable scifb2 interface on iWave RZ/G1H carrier board.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.9.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v5 6/8] drm/panel: Add ilitek ili9341 panel driver
From: Andy Shevchenko @ 2020-05-26 7:46 UTC (permalink / raw)
To: dillon.minfei, Noralf Trønnes
Cc: Linus Walleij, Mark Brown, devicetree, linux-arm Mailing List,
Linux Kernel Mailing List, linux-spi, linux-stm32, dri-devel,
linux-clk
In-Reply-To: <1590378348-8115-7-git-send-email-dillon.minfei@gmail.com>
On Mon, May 25, 2020 at 6:46 AM <dillon.minfei@gmail.com> wrote:
>
> From: dillon min <dillon.minfei@gmail.com>
>
> This driver combine tiny/ili9341.c mipi_dbi_interface driver
> with mipi_dpi_interface driver, can support ili9341 with serial
> mode or parallel rgb interface mode by register configuration.
Noralf told once that this driver should be unified with mi0283qt.c.
So, what should we do here?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Lee Jones @ 2020-05-26 7:24 UTC (permalink / raw)
To: Michael Walle
Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Jean Delvare, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
Greg Kroah-Hartman, linux-gpio, devicetree, linux-kernel,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm-kernel
In-Reply-To: <159e68b4ce53630ef906b2fcbca925bd@walle.cc>
On Mon, 25 May 2020, Michael Walle wrote:
> Am 2020-05-15 12:28, schrieb Lee Jones:
> > On Thu, 30 Apr 2020, Michael Walle wrote:
> >
> > > Hi Lee,
> > >
> > > Am 2020-04-23 19:45, schrieb Michael Walle:
> > > > There might be multiple children with the device tree compatible, for
> > > > example if a MFD has multiple instances of the same function. In this
> > > > case only the first is matched and the other children get a wrong
> > > > of_node reference.
> > > > Add a new option to match also against the unit address of the child
> > > > node. Additonally, a new helper OF_MFD_CELL_REG is added.
> > >
> > >
> > > Do you think this is feasible? I guess this is the biggest uncertainty
> > > for me at the moment in this patch series.
> >
> > I think it sounds fine in principle. So long as it doesn't change the
> > existing behaviour when of_reg isn't set.
> >
> > > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > > ---
> > > > drivers/mfd/mfd-core.c | 29 ++++++++++++++++++++---------
> > > > include/linux/mfd/core.h | 26 ++++++++++++++++++++------
> > > > 2 files changed, 40 insertions(+), 15 deletions(-)
[...]
> > > > diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> > > > index d01d1299e49d..c2c0ad6b14f3 100644
> > > > --- a/include/linux/mfd/core.h
> > > > +++ b/include/linux/mfd/core.h
> > > > @@ -13,8 +13,11 @@
> > > > #include <linux/platform_device.h>
> > > >
> > > > #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
> > > > +#define MFD_OF_REG_VALID BIT(31)
> >
> > What about 64bit platforms?
>
> The idea was to have this as a logical number. I.e. for now you may only
> have one subdevice per unique compatible string. In fact, if you have a
> look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
> subdevices. But there is only one DT node for all three of it. I guess
> this works as long as you don't use phandles to reference the pwm node
> in the device tree. Or you don't want to use device tree properties
> per subdevice (for example the "timeout-sec" of a watchdog device).
>
> So to circumvent this, I thought of having the unit-address (and thus
> the "reg" property) to differentiate between multiple subdevices. Now
> there is one special case for me: this board management controller
> might be upgradable and it might change internally. Thus I came up
> with that logical numbering of subdevices. Rob doesn't seem to be a
> fan of that, though. Therefore, having bit 31 as a valid indicator
> leaves you with 2^31 logical devices, which should be enough ;)
>
> Rob proposed to have the internal offset as the unit-address. But
> in that case I can also use devm_of_platform_populate() and don't
> need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
> individual subdevice driver. But like I said, I wanted to keep the
> internal offsets out of the device tree.
Oh, I see what you're doing.
So you're adding an arbitrary ID to the device's reg property in DT?
How is this not a hack?
Why don't you use the full address for identification?
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
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