* Re: [PATCH 09/12] devfreq: add mediatek cci devfreq
From: Chanwoo Choi @ 2020-05-28 8:00 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: linux-pm, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, srv_heupstream
In-Reply-To: <c39e4f30-805a-78c4-b1c4-e55a03e2408e@samsung.com>
Hi Andrew-sh.Cheng,
On 5/28/20 4:35 PM, Chanwoo Choi wrote:
> Hi Andrew-sh.Cheng,
>
> On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
>> This adds a devfreq driver for the Cache Coherent Interconnect (CCI)
>> of the Mediatek MT8183.
>>
>> On the MT8183 the CCI is supplied by the same regulator as the LITTLE
>> cores. The driver is notified when the regulator voltage changes
>> (driven by cpufreq) and adjusts the CCI frequency to the maximum
>> possible value.
I understood that the mt8183-cci.c and mt8183 cpufreq driver (ARM_MEDIATEK_CPUFREQ)
shared the same regulator. So, when mt8183 cpufreq driver
changes the CPU frequency and voltage, the mt8183-cci.c
changes the CCI frequency according to the new mt8183 frequency
with passive governor.
I think that mt8183-cci.c don't need to change the voltage
because already mt8183 cpufreq changed the voltage of shared regulator.
Why do you change the voltage in this driver?
>>
>> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> ---
>> drivers/devfreq/Kconfig | 10 ++
>> drivers/devfreq/Makefile | 1 +
>> drivers/devfreq/mt8183-cci-devfreq.c | 206 +++++++++++++++++++++++++++++++++++
>
> The mt8183-cci.c is enough for driver name.
>
>> 3 files changed, 217 insertions(+)
>> create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index d9067950af6a..4ed7116271ee 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -103,6 +103,16 @@ config ARM_IMX8M_DDRC_DEVFREQ
>> This adds the DEVFREQ driver for the i.MX8M DDR Controller. It allows
>> adjusting DRAM frequency.
>>
>> +config ARM_MT8183_CCI_DEVFREQ
>> + tristate "MT8183 CCI DEVFREQ Driver"
>> + depends on ARM_MEDIATEK_CPUFREQ
>> + help
>> + This adds a devfreq driver for Cache Coherent Interconnect
>> + of Mediatek MT8183, which is shared the same regulator
>> + with cpu cluster.
>> + It can track buck voltage and update a proper cci frequency.
>
> s/cci/CCI
>
>> + Use notification to get regulator status.
>> +
>> config ARM_TEGRA_DEVFREQ
>> tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>> depends on ARCH_TEGRA_3x_SOC || ARCH_TEGRA_114_SOC || \
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 3eb4d5e6635c..5b1b670c954d 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
>> # DEVFREQ Drivers
>> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
>> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o
>> +obj-$(CONFIG_ARM_MT8183_CCI_DEVFREQ) += mt8183-cci-devfreq.o
>> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
>> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o
>> obj-$(CONFIG_ARM_TEGRA20_DEVFREQ) += tegra20-devfreq.o
>> diff --git a/drivers/devfreq/mt8183-cci-devfreq.c b/drivers/devfreq/mt8183-cci-devfreq.c
>> new file mode 100644
>> index 000000000000..cd7929a83bf8
>> --- /dev/null
>> +++ b/drivers/devfreq/mt8183-cci-devfreq.c
>> @@ -0,0 +1,206 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019 MediaTek Inc.
>
> s/2019/2020
>
>> +
>> + * Author: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/time.h>
>> +
>> +#include "governor.h"
>
> It is not needed. Please remove it.
>
>> +
>> +#define MAX_VOLT_LIMIT (1150000)
>> +
>> +struct cci_devfreq {
>> + struct devfreq *devfreq;
>> + struct regulator *proc_reg;
>
> 'proc' means the 'processor'?
> Instead of 'proc_reg', you better to use 'cpu_reg'.
>
>> + struct clk *cci_clk;
>> + int old_vproc;
>> + unsigned long old_freq;
>> +};
>> +
>> +static int mtk_cci_set_voltage(struct cci_devfreq *cci_df, int vproc)
>> +{
>> + int ret;
>> +
>> + ret = regulator_set_voltage(cci_df->proc_reg, vproc,
>> + MAX_VOLT_LIMIT);
>> + if (!ret)
>> + cci_df->old_vproc = vproc;
>> + return ret;
>> +}
>> +
>> +static int mtk_cci_devfreq_target(struct device *dev, unsigned long *freq,
>> + u32 flags)
>> +{
>> + int ret;
>> + struct cci_devfreq *cci_df = dev_get_drvdata(dev);
>> + struct dev_pm_opp *opp;
>> + unsigned long opp_rate, opp_voltage, old_voltage;
>> +
>> + if (!cci_df)
>> + return -EINVAL;
>> +
>> + if (cci_df->old_freq == *freq)
>> + return 0;
>> +
>> + opp_rate = *freq;
>> + opp = dev_pm_opp_find_freq_floor(dev, &opp_rate);
>> + opp_voltage = dev_pm_opp_get_voltage(opp);
>> + dev_pm_opp_put(opp);
>
>
> You can use the helper function for getting the rate
> with devfreq_recommended_opp(). You can refer the following code
> in drivers/devfreq/exynos-bus.c
>
> opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(opp)) {
> dev_err(dev, "failed to get recommended opp instance\n");
> return PTR_ERR(opp);
> }
> dev_pm_opp_put(opp);
>
>> +
>> + old_voltage = cci_df->old_vproc;
>> + if (old_voltage == 0)
>> + old_voltage = regulator_get_voltage(cci_df->proc_reg);
>> +
>> + // scale up: set voltage first then freq
>> + if (opp_voltage > old_voltage) {
>> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
>> + if (ret) {
>> + pr_err("cci: failed to scale up voltage\n");
>> + return ret;
>> + }
>> + }
>> +
>> + ret = clk_set_rate(cci_df->cci_clk, *freq);
>> + if (ret) {
>> + pr_err("%s: failed cci to set rate: %d\n", __func__,
>> + ret);
>> + mtk_cci_set_voltage(cci_df, old_voltage);
>> + return ret;
>> + }
>> +
>> + // scale down: set freq first then voltage
>> + if (opp_voltage < old_voltage) {
>> + ret = mtk_cci_set_voltage(cci_df, opp_voltage);
>> + if (ret) {
>> + pr_err("cci: failed to scale down voltage\n");
>> + clk_set_rate(cci_df->cci_clk, cci_df->old_freq);
>> + return ret;
>> + }
>> + }
>
>
> I recommend that dev_pm_opp_set_rate() and dev_pm_opp_set_regulator()
> instead of 'clk_set_rate' and 'regulator_set_voltage'.
> In the dev_pm_opp_set_rate(), handle the these sequence.
> You can refer the merged patch[1].
>
> [1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
> - PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
>
>
>> +
>> + cci_df->old_freq = *freq;
>> +
>> + return 0;
>> +}
>> +
>> +static struct devfreq_dev_profile cci_devfreq_profile = {
>> + .target = mtk_cci_devfreq_target,
>
> Need to add '.exit' for calling dev_pm_opp_of_remove_table().
> You can refer the merged devfreq patches like exynos_bus.c, imx-bus.c.
>
>> +};
>> +
>> +static int mtk_cci_devfreq_probe(struct platform_device *pdev)
>> +{
>> + struct device *cci_dev = &pdev->dev;
>> + struct cci_devfreq *cci_df;
>> + struct devfreq_passive_data *passive_data;
>> + int ret;
>> +
>> + cci_df = devm_kzalloc(cci_dev, sizeof(*cci_df), GFP_KERNEL);
>> + if (!cci_df)
>> + return -ENOMEM;
>> +
>> + cci_df->cci_clk = devm_clk_get(cci_dev, "cci_clock");
>> + ret = PTR_ERR_OR_ZERO(cci_df->cci_clk);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(cci_dev, "failed to get clock for CCI: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + cci_df->proc_reg = devm_regulator_get_optional(cci_dev, "proc");
>> + ret = PTR_ERR_OR_ZERO(cci_df->proc_reg);
>> + if (ret) {
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(cci_dev, "failed to get regulator for CCI: %d\n",
>> + ret);
>> + return ret;
>> + }
>
> I recommend that use dev_pm_opp_set_regulators.
> You can refer the merged patch[1].
>
> [1] commit 4294a779bd8dff6c65e7e85ffe7a1ea236e92a68
> - PM / devfreq: exynos-bus: Convert to use dev_pm_opp_set_rate()
>
>
>> + ret = regulator_enable(cci_df->proc_reg);
>> + if (ret) {
>> + pr_warn("enable buck for cci fail\n");
>
> Use dev_err instead of 'pr_warn'.
>
>> + return ret;
>> + }
>> +
>> + ret = dev_pm_opp_of_add_table(cci_dev);
>> + if (ret) {
>> + dev_err(cci_dev, "Fail to init CCI OPP table: %d\n", ret);
>
> How about changing the error log as following
> because in this driver, use the 'failed to' sentence for error handling?
>
> failed to get OPP table for CCI:L %d
>
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, cci_df);
>> +
>> + passive_data = devm_kzalloc(cci_dev, sizeof(*passive_data), GFP_KERNEL);
>> + if (!passive_data)
>> + return -ENOMEM;
>
> On this error case, you have to call dev_pm_opp_of_remove_table().
> You better to make the 'err_opp' jump lable and then add 'goto err_opp'.
>
>> +
>> + passive_data->parent_type = CPUFREQ_PARENT_DEV;
>> +
>> + cci_df->devfreq = devm_devfreq_add_device(cci_dev,
>> + &cci_devfreq_profile,
>> + DEVFREQ_GOV_PASSIVE,
>> + passive_data);
>> + if (IS_ERR(cci_df->devfreq)) {
>> + ret = PTR_ERR(cci_df->devfreq);
>> + dev_err(cci_dev, "cannot create cci devfreq device:%d\n", ret);
>> + dev_pm_opp_of_remove_table(cci_dev);
>
> Instead of direct call, use 'goto err_opp'.
>
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int mtk_cci_devfreq_remove(struct platform_device *pdev)
>> +{
>> + struct device *cci_dev = &pdev->dev;
>> + struct cci_devfreq *cci_df;
>> + struct notifier_block *opp_nb;
>> +
>> + cci_df = platform_get_drvdata(pdev);
>> + opp_nb = &cci_df->opp_nb;
>> +
>> + dev_pm_opp_unregister_notifier(cci_dev, opp_nb);
>
> This patch doesn't call the dev_pm_opp_register_notifier.
> Please remove it.
>
>> + devm_devfreq_remove_device(cci_dev, cci_df->devfreq);
>
> Don't need to call this function because you used devm_devfreq_add_device().
>
>> + dev_pm_opp_of_remove_table(cci_dev)> + regulator_disable(cci_df->proc_reg);
>> +
>> + return 0;
>> +}
>> +
>> +static const __maybe_unused struct of_device_id
>> + mediatek_cci_devfreq_of_match[] = {
>
> Make it on one line and remove '__maybe_unused' keyword.
> - mediatek_cci_devfreq_of_match-> mediatek_cci_of_match
>
>> + { .compatible = "mediatek,mt8183-cci" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, mediatek_cci_devfreq_of_match);
>
> ditto.
>
>> +
>> +static struct platform_driver cci_devfreq_driver = {
>> + .probe = mtk_cci_devfreq_probe,
>> + .remove = mtk_cci_devfreq_remove,
>> + .driver = {
>> + .name = "mediatek-cci-devfreq",
>> + .of_match_table = of_match_ptr(mediatek_cci_devfreq_of_match),
>
> ditto.
>
>> + },
>> +};
>> +
>> +static int __init mtk_cci_devfreq_init(void)
>> +{
>> + return platform_driver_register(&cci_devfreq_driver);
>> +}
>> +module_init(mtk_cci_devfreq_init)
>> +
>> +static void __exit mtk_cci_devfreq_exit(void)
>> +{
>> + platform_driver_unregister(&cci_devfreq_driver);
>> +}
>> +module_exit(mtk_cci_devfreq_exit)
>
> Use 'module_platform_driver' instead of module_init and module_exit.
>
>> +
>> +MODULE_DESCRIPTION("Mediatek CCI devfreq driver");
>> +MODULE_AUTHOR("Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>");
>> +MODULE_LICENSE("GPL v2");
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Dongchun Zhu @ 2020-05-28 8:04 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, Linus Walleij, Bartosz Golaszewski,
Mauro Carvalho Chehab, Andy Shevchenko, Mark Rutland,
Nicolas Boichat, Tomasz Figa, Matthias Brugger, Cao Bing Bu,
srv_heupstream, moderated list:ARM/Mediatek SoC support,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Sj Huang,
Linux Media Mailing List, devicetree, Louis Kuo,
Shengnan Wang (王圣男)
In-Reply-To: <20200528072332.GW7618@paasikivi.fi.intel.com>
Hi Sakari,
On Thu, 2020-05-28 at 10:23 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> On Thu, May 28, 2020 at 11:34:42AM +0800, Dongchun Zhu wrote:
> > Hi Sakari, Rob,
> >
> > On Thu, 2020-05-28 at 00:16 +0300, Sakari Ailus wrote:
> > > Hi Rob, Dongchun,
> > >
> > > On Wed, May 27, 2020 at 09:27:22AM -0600, Rob Herring wrote:
> > > > > > > + properties:
> > > > > > > + endpoint:
> > > > > > > + type: object
> > > > > > > + additionalProperties: false
> > > > > > > +
> > > > > > > + properties:
> > > > >
> > > > > Actually I wonder whether we need to declare 'clock-lanes' here?
> > > >
> > > > Yes, if you are using it.
> > >
> > > Dongchun, can you confirm the chip has a single data and a single clock
> > > lane and that it does not support lane reordering?
> > >
> >
> > From the datasheet, 'MIPI inside the OV02A10 provides one single
> > uni-directional clock lane and one bi-directional data lane solution for
> > communication links between components inside a mobile device.
> > The data lane has full support for HS(uni-directional) and
> > LP(bi-directional) data transfer mode.'
> >
> > The sensor doesn't support lane reordering, so 'clock-lanes' property
> > would not be added in next release.
> >
> > > So if there's nothing to convey to the driver, also the data-lanes should
> > > be removed IMO.
> > >
> >
> > However, 'data-lanes' property may still be required.
> > It is known that either data-lanes or clock-lanes is an array of
> > physical data lane indexes. Position of an entry determines the logical
> > lane number, while the value of an entry indicates physical lane, e.g.,
> > for 1-lane MIPI CSI-2 bus we could have "data-lanes = <1>;", assuming
> > the clock lane is on hardware lane 0.
> >
> > As mentioned earlier, the OV02A10 sensor supports only 1C1D and does not
> > support lane reordering, so here we shall use 'data-lanes = <1>' as
> > there is only a clock lane for OV02A10.
> >
> > Reminder:
> > If 'data-lanes' property is not present, the driver would assume
> > four-lane operation. This means for one-lane or two-lane operation, this
> > property must be present and set to the right physical lane indexes.
> > If the hardware does not support lane reordering, monotonically
> > incremented values shall be used from 0 or 1 onwards, depending on
> > whether or not there is also a clock lane.
>
> How can the driver use four lanes, considering the device only supports a
> single lane??
>
I understood your meaning.
If we omit the property 'data-lanes', the sensor should work still.
But then what's the meaning of the existence of 'data-lanes'?
If this property 'data-lanes' is always optional, then why dt-bindings
provide the interface?
In the meantime, if omitting 'data-lanes' for one sensor(transmitter)
that has only one physical data lane, MIPI receiver(e.g., MIPI CSI-2)
shall enable four-lane configuration, which may increase consumption of
both power and resource in the process of IIC communication.
^ permalink raw reply
* Re: [Patch 2/2] media: ti-vpe: Add the VIP driver
From: Hans Verkuil @ 2020-05-28 8:09 UTC (permalink / raw)
To: Benoit Parrot
Cc: Rob Herring, linux-media, devicetree, linux-kernel,
Nikhil Devshatwar
In-Reply-To: <20200526215708.3vp4z4qjzpj66t36@ti.com>
On 26/05/2020 23:57, Benoit Parrot wrote:
> Hans,
>
> Thanks for the review.
>
> Hans Verkuil <hverkuil@xs4all.nl> wrote on Tue [2020-May-26 13:48:35 +0200]:
>> On 23/05/2020 00:54, Benoit Parrot wrote:
>>> VIP stands for Video Input Port, it can be found on devices such as
>>> DRA7xx and provides a parallel interface to a video source such as
>>> a sensor or TV decoder. Each VIP can support two inputs (slices) and
>>> a SoC can be configured with a variable number of VIP's.
>>> Each slice can supports two ports each connected to its own
>>> sub-device.
>>>
>>> Signed-off-by: Benoit Parrot <bparrot@ti.com>
>>> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
>>> ---
>>> drivers/media/platform/Kconfig | 13 +
>>> drivers/media/platform/ti-vpe/Makefile | 2 +
>>> drivers/media/platform/ti-vpe/vip.c | 4158 ++++++++++++++++++++++++
>>> drivers/media/platform/ti-vpe/vip.h | 724 +++++
>>> 4 files changed, 4897 insertions(+)
>>> create mode 100644 drivers/media/platform/ti-vpe/vip.c
>>> create mode 100644 drivers/media/platform/ti-vpe/vip.h
>>>
<snip>
>>> +static int vip_enum_fmt_vid_cap(struct file *file, void *priv,
>>> + struct v4l2_fmtdesc *f)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct vip_port *port = stream->port;
>>> + struct vip_fmt *fmt;
>>> +
>>> + vip_dbg(3, stream, "enum_fmt index:%d\n", f->index);
>>> + if (f->index >= port->num_active_fmt)
>>> + return -EINVAL;
>>> +
>>> + fmt = port->active_fmt[f->index];
>>> +
>>> + f->pixelformat = fmt->fourcc;
>>> + f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>
>> No need to set the type field.
>
> Ok.
>
>>
>>> + vip_dbg(3, stream, "enum_fmt fourcc:%s\n",
>>> + fourcc_to_str(f->pixelformat));
>>
>> Excessive debugging.
>
> Why excessive?
Two debug messages for a simple function like this seems overkill.
Besides, the v4l2 core already has debugging support for ioctl calls:
https://linuxtv.org/downloads/v4l-dvb-apis-new/driver-api/v4l2-dev.html#video-device-debugging
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int vip_enum_framesizes(struct file *file, void *priv,
>>> + struct v4l2_frmsizeenum *f)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct vip_port *port = stream->port;
>>> + struct vip_fmt *fmt;
>>> + struct v4l2_subdev_frame_size_enum fse;
>>> + int ret;
>>> +
>>> + fmt = find_port_format_by_pix(port, f->pixel_format);
>>> + if (!fmt)
>>> + return -EINVAL;
>>> +
>>> + fse.index = f->index;
>>> + fse.pad = port->source_pad;
>>> + fse.code = fmt->code;
>>> + fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>>> + ret = v4l2_subdev_call(port->subdev, pad, enum_frame_size, NULL, &fse);
>>> + if (ret == -ENOIOCTLCMD && !f->index) {
>>> + /*
>>> + * if subdev does not support enum_frame_size
>>> + * then use get_fmt
>>
>> I don't think that's right. If the subdev doesn't support this, then
>> this ioctl should be disabled altogether. Typically this ioctl is only
>> valid for sensor subdevs, not for video receivers.
>>
>> Use v4l2_subdev_has_op() and v4l2_disable_ioctl().
>
> You mean to check if the subdev support this ioctl and if not disable it
> for the current video device only, correct?
Correct.
There are several other drivers that do this, just grep for them.
>>> +static int vip_calc_format_size(struct vip_port *port,
>>> + struct vip_fmt *fmt,
>>> + struct v4l2_format *f)
>>> +{
>>> + enum v4l2_field *field;
>>> + unsigned int stride;
>>> +
>>> + if (!fmt) {
>>> + vip_dbg(2, port,
>>> + "no vip_fmt format provided!\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + field = &f->fmt.pix.field;
>>> + if (*field == V4L2_FIELD_ANY)
>>> + *field = V4L2_FIELD_NONE;
>>> + else if (V4L2_FIELD_NONE != *field && V4L2_FIELD_ALTERNATE != *field)
>>> + return -EINVAL;
>>> +
>>> + v4l_bound_align_image(&f->fmt.pix.width, MIN_W, MAX_W, W_ALIGN,
>>> + &f->fmt.pix.height, MIN_H, MAX_H, H_ALIGN,
>>> + S_ALIGN);
>>> +
>>> + stride = f->fmt.pix.width * (fmt->vpdma_fmt[0]->depth >> 3);
>>> + if (stride > f->fmt.pix.bytesperline)
>>> + f->fmt.pix.bytesperline = stride;
>>> +
>>> + f->fmt.pix.bytesperline = clamp_t(u32, f->fmt.pix.bytesperline,
>>> + stride, VPDMA_MAX_STRIDE);
>>> + f->fmt.pix.bytesperline = ALIGN(f->fmt.pix.bytesperline,
>>> + VPDMA_STRIDE_ALIGN);
>>> +
>>> + f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
>>> + if (fmt->coplanar) {
>>> + f->fmt.pix.sizeimage += f->fmt.pix.height *
>>> + f->fmt.pix.bytesperline *
>>> + fmt->vpdma_fmt[VIP_CHROMA]->depth >> 3;
>>> + }
>>> +
>>> + f->fmt.pix.colorspace = fmt->colorspace;
>>> + f->fmt.pix.priv = 0;
>>
>> No need to set this.
>
> You mean pix.priv? I thought I remember v4l2-compliance complaining about
> something like this?
Yes, pix.priv. It used to complain a long time ago, but the v4l2 core should
handle this. Basically drivers shouldn't touch pix.priv.
>
>>
>>> +
>>> + vip_dbg(3, port, "calc_format_size: fourcc:%s size: %dx%d bpl:%d img_size:%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + f->fmt.pix.width, f->fmt.pix.height,
>>> + f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline bool vip_is_size_dma_aligned(u32 bpp, u32 width)
>>> +{
>>> + return ((width * bpp) == ALIGN(width * bpp, VPDMA_STRIDE_ALIGN));
>>> +}
>>> +
>>> +static int vip_try_fmt_vid_cap(struct file *file, void *priv,
>>> + struct v4l2_format *f)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct vip_port *port = stream->port;
>>> + struct v4l2_subdev_frame_size_enum fse;
>>> + struct vip_fmt *fmt;
>>> + u32 best_width, best_height, largest_width, largest_height;
>>> + int ret, found;
>>> + enum vip_csc_state csc_direction;
>>> +
>>> + vip_dbg(3, stream, "try_fmt fourcc:%s size: %dx%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + f->fmt.pix.width, f->fmt.pix.height);
>>> +
>>> + fmt = find_port_format_by_pix(port, f->fmt.pix.pixelformat);
>>> + if (!fmt) {
>>> + vip_dbg(2, stream,
>>> + "Fourcc format (0x%08x) not found.\n",
>>> + f->fmt.pix.pixelformat);
>>> +
>>> + /* Just get the first one enumerated */
>>> + fmt = port->active_fmt[0];
>>> + f->fmt.pix.pixelformat = fmt->fourcc;
>>> + }
>>> +
>>> + csc_direction = vip_csc_direction(fmt->code, fmt->finfo);
>>> + if (csc_direction != VIP_CSC_NA) {
>>> + if (!is_csc_available(port)) {
>>> + vip_dbg(2, stream,
>>> + "CSC not available for Fourcc format (0x%08x).\n",
>>> + f->fmt.pix.pixelformat);
>>> +
>>> + /* Just get the first one enumerated */
>>> + fmt = port->active_fmt[0];
>>> + f->fmt.pix.pixelformat = fmt->fourcc;
>>> + /* re-evaluate the csc_direction here */
>>> + csc_direction = vip_csc_direction(fmt->code,
>>> + fmt->finfo);
>>> + } else {
>>> + vip_dbg(3, stream, "CSC active on Port %c: going %s\n",
>>> + port->port_id == VIP_PORTA ? 'A' : 'B',
>>> + (csc_direction == VIP_CSC_Y2R) ? "Y2R" : "R2Y");
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * Given that sensors might support multiple mbus code we need
>>> + * to use the one that matches the requested pixel format
>>> + */
>>> + port->try_mbus_framefmt = port->mbus_framefmt;
>>> + port->try_mbus_framefmt.code = fmt->code;
>>> +
>>> + /* check for/find a valid width/height */
>>> + ret = 0;
>>> + found = false;
>>> + best_width = 0;
>>> + best_height = 0;
>>> + largest_width = 0;
>>> + largest_height = 0;
>>> + fse.pad = port->source_pad;
>>> + fse.code = fmt->code;
>>> + fse.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>>> + for (fse.index = 0; ; fse.index++) {
>>> + u32 bpp = fmt->vpdma_fmt[0]->depth >> 3;
>>> +
>>> + ret = v4l2_subdev_call(port->subdev, pad,
>>> + enum_frame_size, NULL, &fse);
>>> + if (ret == -ENOIOCTLCMD) {
>>> + /*
>>> + * if subdev does not support enum_frame_size
>>> + * then just try to set_fmt directly
>>> + */
>>> + struct v4l2_subdev_format format = {
>>> + .which = V4L2_SUBDEV_FORMAT_TRY,
>>> + };
>>> + struct v4l2_subdev_pad_config *pad_cfg;
>>> +
>>> + pad_cfg = v4l2_subdev_alloc_pad_config(port->subdev);
>>> + if (!pad_cfg)
>>> + return -ENOMEM;
>>> +
>>> + v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
>>> + fmt->code);
>>> + ret = v4l2_subdev_call(port->subdev, pad, set_fmt,
>>> + pad_cfg, &format);
>>> + if (ret)
>>> + /* here regardless of the reason we give up */
>>> + break;
>>> +
>>> + if (f->fmt.pix.width == format.format.width &&
>>> + f->fmt.pix.height == format.format.height) {
>>> + found = true;
>>> + vip_dbg(3, stream, "try_fmt loop:%d found direct match: %dx%d\n",
>>> + fse.index, format.format.width,
>>> + format.format.height);
>>> + }
>>> + largest_width = format.format.width;
>>> + largest_height = format.format.height;
>>> + best_width = format.format.width;
>>> + best_height = format.format.height;
>>> +
>>> + v4l2_subdev_free_pad_config(pad_cfg);
>>> + break;
>>> +
>>> + } else if (ret) {
>>> + break;
>>> + }
>>> +
>>> + vip_dbg(3, stream, "try_fmt loop:%d fourcc:%s size: %dx%d\n",
>>> + fse.index, fourcc_to_str(f->fmt.pix.pixelformat),
>>> + fse.max_width, fse.max_height);
>>> +
>>> + if (!vip_is_size_dma_aligned(bpp, fse.max_width))
>>> + continue;
>>> +
>>> + if ((fse.max_width >= largest_width) &&
>>> + (fse.max_height >= largest_height)) {
>>> + vip_dbg(3, stream, "try_fmt loop:%d found new larger: %dx%d\n",
>>> + fse.index, fse.max_width, fse.max_height);
>>> + largest_width = fse.max_width;
>>> + largest_height = fse.max_height;
>>> + }
>>> +
>>> + if ((fse.max_width >= f->fmt.pix.width) &&
>>> + (fse.max_height >= f->fmt.pix.height)) {
>>> + vip_dbg(3, stream, "try_fmt loop:%d found at least larger: %dx%d\n",
>>> + fse.index, fse.max_width, fse.max_height);
>>> +
>>> + if (!best_width ||
>>> + ((abs(best_width - f->fmt.pix.width) >=
>>> + abs(fse.max_width - f->fmt.pix.width)) &&
>>> + (abs(best_height - f->fmt.pix.height) >=
>>> + abs(fse.max_height - f->fmt.pix.height)))) {
>>> + best_width = fse.max_width;
>>> + best_height = fse.max_height;
>>> + vip_dbg(3, stream, "try_fmt loop:%d found new best: %dx%d\n",
>>> + fse.index, fse.max_width,
>>> + fse.max_height);
>>> + }
>>> + }
>>> +
>>> + if ((f->fmt.pix.width == fse.max_width) &&
>>> + (f->fmt.pix.height == fse.max_height)) {
>>> + found = true;
>>> + vip_dbg(3, stream, "try_fmt loop:%d found direct match: %dx%d\n",
>>> + fse.index, fse.max_width,
>>> + fse.max_height);
>>> + break;
>>> + }
>>> +
>>> + if ((f->fmt.pix.width >= fse.min_width) &&
>>> + (f->fmt.pix.width <= fse.max_width) &&
>>> + (f->fmt.pix.height >= fse.min_height) &&
>>> + (f->fmt.pix.height <= fse.max_height)) {
>>> + found = true;
>>> + vip_dbg(3, stream, "try_fmt loop:%d found direct range match: %dx%d\n",
>>> + fse.index, fse.max_width,
>>> + fse.max_height);
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (found) {
>>> + port->try_mbus_framefmt.width = f->fmt.pix.width;
>>> + port->try_mbus_framefmt.height = f->fmt.pix.height;
>>> + /* No need to check for scaling */
>>> + goto calc_size;
>>> + } else if (largest_width && f->fmt.pix.width > largest_width) {
>>> + port->try_mbus_framefmt.width = largest_width;
>>> + port->try_mbus_framefmt.height = largest_height;
>>> + } else if (best_width) {
>>> + port->try_mbus_framefmt.width = best_width;
>>> + port->try_mbus_framefmt.height = best_height;
>>> + } else {
>>> + /* use existing values as default */
>>> + }
>>> +
>>> + vip_dbg(3, stream, "try_fmt best subdev size: %dx%d\n",
>>> + port->try_mbus_framefmt.width,
>>> + port->try_mbus_framefmt.height);
>>> +
>>> + if (is_scaler_available(port) &&
>>> + csc_direction != VIP_CSC_Y2R &&
>>> + !vip_is_mbuscode_raw(fmt->code) &&
>>> + f->fmt.pix.height <= port->try_mbus_framefmt.height &&
>>> + port->try_mbus_framefmt.height <= SC_MAX_PIXEL_HEIGHT &&
>>> + port->try_mbus_framefmt.width <= SC_MAX_PIXEL_WIDTH) {
>>> + /*
>>> + * Scaler is only accessible if the dst colorspace is YUV.
>>> + * As the input to the scaler must be in YUV mode only.
>>> + *
>>> + * Scaling up is allowed only horizontally.
>>> + */
>>> + unsigned int hratio, vratio, width_align, height_align;
>>> + u32 bpp = fmt->vpdma_fmt[0]->depth >> 3;
>>> +
>>> + vip_dbg(3, stream, "Scaler active on Port %c: requesting %dx%d\n",
>>> + port->port_id == VIP_PORTA ? 'A' : 'B',
>>> + f->fmt.pix.width, f->fmt.pix.height);
>>> +
>>> + /* Just make sure everything is properly aligned */
>>> + width_align = ALIGN(f->fmt.pix.width * bpp, VPDMA_STRIDE_ALIGN);
>>> + width_align /= bpp;
>>> + height_align = ALIGN(f->fmt.pix.height, 2);
>>> +
>>> + f->fmt.pix.width = width_align;
>>> + f->fmt.pix.height = height_align;
>>> +
>>> + hratio = f->fmt.pix.width * 1000 /
>>> + port->try_mbus_framefmt.width;
>>> + vratio = f->fmt.pix.height * 1000 /
>>> + port->try_mbus_framefmt.height;
>>> + if (hratio < 125) {
>>> + f->fmt.pix.width = port->try_mbus_framefmt.width / 8;
>>> + vip_dbg(3, stream, "Horizontal scaling ratio out of range adjusting -> %d\n",
>>> + f->fmt.pix.width);
>>> + }
>>> +
>>> + if (vratio < 188) {
>>> + f->fmt.pix.height = port->try_mbus_framefmt.height / 4;
>>> + vip_dbg(3, stream, "Vertical scaling ratio out of range adjusting -> %d\n",
>>> + f->fmt.pix.height);
>>> + }
>>> + vip_dbg(3, stream, "Scaler: got %dx%d\n",
>>> + f->fmt.pix.width, f->fmt.pix.height);
>>> + } else {
>>> + /* use existing values as default */
>>> + f->fmt.pix.width = port->try_mbus_framefmt.width;
>>> + f->fmt.pix.height = port->try_mbus_framefmt.height;
>>> + }
>>> +
>>> +calc_size:
>>> + /* That we have a fmt calculate imagesize and bytesperline */
>>> + return vip_calc_format_size(port, fmt, f);
>>> +}
>>> +
>>> +static int vip_g_fmt_vid_cap(struct file *file, void *priv,
>>> + struct v4l2_format *f)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct vip_port *port = stream->port;
>>> + struct vip_fmt *fmt = port->fmt;
>>> +
>>> + /* Use last known values or defaults */
>>> + f->fmt.pix.width = stream->width;
>>> + f->fmt.pix.height = stream->height;
>>> + f->fmt.pix.pixelformat = port->fmt->fourcc;
>>> + f->fmt.pix.field = stream->sup_field;
>>> + f->fmt.pix.colorspace = port->fmt->colorspace;
>>> + f->fmt.pix.bytesperline = stream->bytesperline;
>>> + f->fmt.pix.sizeimage = stream->sizeimage;
>>> +
>>> + vip_dbg(3, stream,
>>> + "g_fmt fourcc:%s code: %04x size: %dx%d bpl:%d img_size:%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + fmt->code,
>>> + f->fmt.pix.width, f->fmt.pix.height,
>>> + f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
>>> + vip_dbg(3, stream, "g_fmt vpdma data type: 0x%02X\n",
>>> + port->fmt->vpdma_fmt[0]->data_type);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int vip_s_fmt_vid_cap(struct file *file, void *priv,
>>> + struct v4l2_format *f)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct vip_port *port = stream->port;
>>> + struct v4l2_subdev_format sfmt;
>>> + struct v4l2_mbus_framefmt *mf;
>>> + enum vip_csc_state csc_direction;
>>> + int ret;
>>> +
>>> + vip_dbg(3, stream, "s_fmt input fourcc:%s size: %dx%d bpl:%d img_size:%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + f->fmt.pix.width, f->fmt.pix.height,
>>> + f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
>>> +
>>> + ret = vip_try_fmt_vid_cap(file, priv, f);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + vip_dbg(3, stream, "s_fmt try_fmt fourcc:%s size: %dx%d bpl:%d img_size:%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + f->fmt.pix.width, f->fmt.pix.height,
>>> + f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
>>> +
>>> + if (vb2_is_busy(&stream->vb_vidq)) {
>>> + vip_err(stream, "%s queue busy\n", __func__);
>>> + return -EBUSY;
>>> + }
>>> +
>>> + /*
>>> + * Check if we need the scaler or not
>>> + *
>>> + * Since on previous S_FMT call the scaler might have been
>>> + * allocated if it is not needed in this instance we will
>>> + * attempt to free it just in case.
>>> + *
>>> + * free_scaler() is harmless unless the current port
>>> + * allocated it.
>>> + */
>>> + if (f->fmt.pix.width == port->try_mbus_framefmt.width &&
>>> + f->fmt.pix.height == port->try_mbus_framefmt.height)
>>> + free_scaler(port);
>>> + else
>>> + allocate_scaler(port);
>>> +
>>> + port->fmt = find_port_format_by_pix(port,
>>> + f->fmt.pix.pixelformat);
>>> + stream->width = f->fmt.pix.width;
>>> + stream->height = f->fmt.pix.height;
>>> + stream->bytesperline = f->fmt.pix.bytesperline;
>>> + stream->sizeimage = f->fmt.pix.sizeimage;
>>> + stream->sup_field = f->fmt.pix.field;
>>> + stream->field = f->fmt.pix.field;
>>> +
>>> + port->c_rect.left = 0;
>>> + port->c_rect.top = 0;
>>> + port->c_rect.width = stream->width;
>>> + port->c_rect.height = stream->height;
>>> +
>>> + /*
>>> + * Check if we need the csc unit or not
>>> + *
>>> + * Since on previous S_FMT call, the csc might have been
>>> + * allocated if it is not needed in this instance we will
>>> + * attempt to free it just in case.
>>> + *
>>> + * free_csc() is harmless unless the current port
>>> + * allocated it.
>>> + */
>>> + csc_direction = vip_csc_direction(port->fmt->code, port->fmt->finfo);
>>> + if (csc_direction == VIP_CSC_NA)
>>> + free_csc(port);
>>> + else
>>> + allocate_csc(port, csc_direction);
>>> +
>>> + if (stream->sup_field == V4L2_FIELD_ALTERNATE)
>>> + port->flags |= FLAG_INTERLACED;
>>> + else
>>> + port->flags &= ~FLAG_INTERLACED;
>>> +
>>> + vip_dbg(3, stream, "s_fmt fourcc:%s size: %dx%d bpl:%d img_size:%d\n",
>>> + fourcc_to_str(f->fmt.pix.pixelformat),
>>> + f->fmt.pix.width, f->fmt.pix.height,
>>> + f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
>>> +
>>> + mf = &sfmt.format;
>>> + v4l2_fill_mbus_format(mf, &f->fmt.pix, port->fmt->code);
>>> + /* Make sure to use the subdev size found in the try_fmt */
>>> + mf->width = port->try_mbus_framefmt.width;
>>> + mf->height = port->try_mbus_framefmt.height;
>>> +
>>> + vip_dbg(3, stream, "s_fmt pix_to_mbus mbus_code: %04X size: %dx%d\n",
>>> + mf->code,
>>> + mf->width, mf->height);
>>> +
>>> + sfmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>>> + sfmt.pad = port->source_pad;
>>> + ret = v4l2_subdev_call(port->subdev, pad, set_fmt, NULL, &sfmt);
>>> + if (ret) {
>>> + vip_dbg(1, stream, "set_fmt failed in subdev\n");
>>> + return ret;
>>> + }
>>> +
>>> + /* Save it */
>>> + port->mbus_framefmt = *mf;
>>> +
>>> + vip_dbg(3, stream, "s_fmt subdev fmt mbus_code: %04X size: %dx%d\n",
>>> + port->mbus_framefmt.code,
>>> + port->mbus_framefmt.width, port->mbus_framefmt.height);
>>> + vip_dbg(3, stream, "s_fmt vpdma data type: 0x%02X\n",
>>> + port->fmt->vpdma_fmt[0]->data_type);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * Does the exact opposite of set_fmt_params
>>> + * It makes sure the DataPath register is sane after tear down
>>> + */
>>> +static void unset_fmt_params(struct vip_stream *stream)
>>> +{
>>> + struct vip_dev *dev = stream->port->dev;
>>> + struct vip_port *port = stream->port;
>>> +
>>> + stream->sequence = 0;
>>> + if (stream->port->flags & FLAG_INTERLACED)
>>> + stream->field = V4L2_FIELD_TOP;
>>> +
>>> + if (port->csc == VIP_CSC_Y2R) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev, VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_SRC_DATA_SELECT, 0);
>>> + } else {
>>> + vip_set_slice_path(dev, VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> + /* We are done */
>>> + return;
>>> + } else if (port->csc == VIP_CSC_R2Y) {
>>> + if (port->scaler && port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + }
>>> + } else if (port->scaler) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + }
>>> + } else if (port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + }
>>> + } else {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + }
>>> + }
>>> + /* We are done */
>>> + return;
>>> + } else if (v4l2_is_format_rgb(port->fmt->finfo)) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> + /* We are done */
>>> + return;
>>> + }
>>> +
>>> + if (port->scaler && port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + }
>>> + } else if (port->scaler) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_2_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + }
>>> + } else if (port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_2_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> + } else {
>>> + /*
>>> + * We undo all data path setting except for the multi
>>> + * stream case.
>>> + * Because we cannot disrupt other on-going capture if only
>>> + * one stream is terminated the other might still be going
>>> + */
>>> + vip_set_slice_path(dev, VIP_MULTI_CHANNEL_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * Set the registers that are modified when the video format changes.
>>> + */
>>> +static void set_fmt_params(struct vip_stream *stream)
>>> +{
>>
>> Hmm, this is a *very* long function. Perhaps this could be split up a bit,
>> or reorganized?
>
> Yeah, I'll start by removing the extra comment lines and reformat it.
>
>>
>>> + struct vip_dev *dev = stream->port->dev;
>>> + struct vip_port *port = stream->port;
>>> +
>>> + stream->sequence = 0;
>>> + if (stream->port->flags & FLAG_INTERLACED)
>>> + stream->field = V4L2_FIELD_TOP;
>>> +
>>> + if (port->csc == VIP_CSC_Y2R) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + /* Set alpha component in background color */
>>> + vpdma_set_bg_color(dev->shared->vpdma,
>>> + (struct vpdma_data_format *)
>>> + port->fmt->vpdma_fmt[0],
>>> + 0xff);
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: YUV422
>>> + * Output: Y_UP/UV_UP: RGB
>>> + * CSC_SRC_SELECT = 1
>>> + * RGB_OUT_HI_SELECT = 1
>>> + * RGB_SRC_SELECT = 1
>>> + * MULTI_CHANNEL_SELECT = 0
>>
>> It's a bit pointless to comment what the register values should be when you
>> set them in the code below. I'd drop that part, it will make the code
>> shorter.
>
> Ok.
>
>>
>>> + */
>>> + vip_set_slice_path(dev, VIP_CSC_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>
>> For readability purposes I think it is better to keep this on one line. Same for
>> the other vip_set_slice_path calls.
>
> Ok.
>
>>
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_SRC_DATA_SELECT, 1);
>>> + } else {
>>> + /*
>>> + * Input B: YUV422
>>> + * Output: Y_UP/UV_UP: RGB
>>> + * CSC_SRC_SELECT = 2
>>> + * RGB_OUT_LO_SELECT = 1
>>> + * MULTI_CHANNEL_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_CSC_SRC_DATA_SELECT, 2);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 1);
>>> + }
>>> + /* We are done */
>>> + return;
>>> + } else if (port->csc == VIP_CSC_R2Y) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + if (port->scaler && port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: RGB
>>> + * Output: Y_UP/UV_UP: Scaled YUV420
>>> + * CSC_SRC_SELECT = 4
>>> + * SC_SRC_SELECT = 1
>>> + * CHR_DS_1_SRC_SELECT = 1
>>> + * CHR_DS_1_BYPASS = 0
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 4);
>>> + vip_set_slice_path(dev,
>>> + VIP_SC_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + } else {
>>> + vip_err(stream, "RGB sensor can only be on Port A\n");
>>> + }
>>> + } else if (port->scaler) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: RGB
>>> + * Output: Y_UP: Scaled YUV422
>>> + * CSC_SRC_SELECT = 4
>>> + * SC_SRC_SELECT = 1
>>> + * CHR_DS_1_SRC_SELECT = 1
>>> + * CHR_DS_1_BYPASS = 1
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 4);
>>> + vip_set_slice_path(dev,
>>> + VIP_SC_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + } else {
>>> + vip_err(stream, "RGB sensor can only be on Port A\n");
>>> + }
>>> + } else if (port->fmt->coplanar) {
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: RGB
>>> + * Output: Y_UP/UV_UP: YUV420
>>> + * CSC_SRC_SELECT = 4
>>> + * CHR_DS_1_SRC_SELECT = 2
>>> + * CHR_DS_1_BYPASS = 0
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 4);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 2);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + } else {
>>> + vip_err(stream, "RGB sensor can only be on Port A\n");
>>> + }
>>> + } else {
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: RGB
>>> + * Output: Y_UP/UV_UP: YUV420
>>> + * CSC_SRC_SELECT = 4
>>> + * CHR_DS_1_SRC_SELECT = 2
>>> + * CHR_DS_1_BYPASS = 1
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CSC_SRC_DATA_SELECT, 4);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT,
>>> + 2);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_DATA_BYPASS, 1);
>>> + vip_set_slice_path(dev,
>>> + VIP_RGB_OUT_HI_DATA_SELECT,
>>> + 0);
>>> + } else {
>>> + vip_err(stream, "RGB sensor can only be on Port A\n");
>>> + }
>>> + }
>>> + /* We are done */
>>> + return;
>>> + } else if (v4l2_is_format_rgb(port->fmt->finfo)) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + /* Set alpha component in background color */
>>> + vpdma_set_bg_color(dev->shared->vpdma,
>>> + (struct vpdma_data_format *)
>>> + port->fmt->vpdma_fmt[0],
>>> + 0xff);
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: RGB
>>> + * Output: Y_LO/UV_LO: RGB
>>> + * RGB_OUT_LO_SELECT = 1
>>> + * MULTI_CHANNEL_SELECT = 1
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 1);
>>> + } else {
>>> + vip_err(stream, "RGB sensor can only be on Port A\n");
>>> + }
>>> + /* We are done */
>>> + return;
>>> + }
>>> +
>>> + if (port->scaler && port->fmt->coplanar) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: YUV422
>>> + * Output: Y_UP/UV_UP: Scaled YUV420
>>> + * SC_SRC_SELECT = 2
>>> + * CHR_DS_1_SRC_SELECT = 1
>>> + * CHR_DS_1_BYPASS = 0
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 2);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + /*
>>> + * Input B: YUV422
>>> + * Output: Y_LO/UV_LO: Scaled YUV420
>>> + * SC_SRC_SELECT = 3
>>> + * CHR_DS_2_SRC_SELECT = 1
>>> + * RGB_OUT_LO_SELECT = 0
>>> + * MULTI_CHANNEL_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 3);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + }
>>> + } else if (port->scaler) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: YUV422
>>> + * Output: Y_UP: Scaled YUV422
>>> + * SC_SRC_SELECT = 2
>>> + * CHR_DS_1_SRC_SELECT = 1
>>> + * CHR_DS_1_BYPASS = 1
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 2);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + /*
>>> + * Input B: YUV422
>>> + * Output: UV_UP: Scaled YUV422
>>> + * SC_SRC_SELECT = 3
>>> + * CHR_DS_2_SRC_SELECT = 1
>>> + * CHR_DS_1_BYPASS = 1
>>> + * CHR_DS_2_BYPASS = 1
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_SC_SRC_DATA_SELECT, 3);
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 1);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_2_DATA_BYPASS, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + }
>>> + } else if (port->fmt->coplanar) {
>>> + port->flags &= ~FLAG_MULT_PORT;
>>> + if (port->port_id == VIP_PORTA) {
>>> + /*
>>> + * Input A: YUV422
>>> + * Output: Y_UP/UV_UP: YUV420
>>> + * CHR_DS_1_SRC_SELECT = 3
>>> + * CHR_DS_1_BYPASS = 0
>>> + * RGB_OUT_HI_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_1_SRC_DATA_SELECT, 3);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_1_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_HI_DATA_SELECT, 0);
>>> + } else {
>>> + /*
>>> + * Input B: YUV422
>>> + * Output: Y_LO/UV_LO: YUV420
>>> + * CHR_DS_2_SRC_SELECT = 4
>>> + * CHR_DS_2_BYPASS = 0
>>> + * RGB_OUT_LO_SELECT = 0
>>> + * MULTI_CHANNEL_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev,
>>> + VIP_CHR_DS_2_SRC_DATA_SELECT, 4);
>>> + vip_set_slice_path(dev, VIP_CHR_DS_2_DATA_BYPASS, 0);
>>> + vip_set_slice_path(dev,
>>> + VIP_MULTI_CHANNEL_DATA_SELECT, 0);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> + } else {
>>> + port->flags |= FLAG_MULT_PORT;
>>> + /*
>>> + * Input A/B: YUV422
>>> + * Output: Y_LO: YUV422 - UV_LO: YUV422
>>> + * MULTI_CHANNEL_SELECT = 1
>>> + * RGB_OUT_LO_SELECT = 0
>>> + */
>>> + vip_set_slice_path(dev, VIP_MULTI_CHANNEL_DATA_SELECT, 1);
>>> + vip_set_slice_path(dev, VIP_RGB_OUT_LO_DATA_SELECT, 0);
>>> + }
>>> +}
>>> +
>>> +static int vip_g_selection(struct file *file, void *fh,
>>> + struct v4l2_selection *s)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> +
>>> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + switch (s->target) {
>>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>>> + s->r.left = 0;
>>> + s->r.top = 0;
>>> + s->r.width = stream->width;
>>> + s->r.height = stream->height;
>>> + return 0;
>>> +
>>> + case V4L2_SEL_TGT_COMPOSE:
>>> + case V4L2_SEL_TGT_CROP:
>>> + s->r = stream->port->c_rect;
>>> + return 0;
>>> + }
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static int enclosed_rectangle(struct v4l2_rect *a, struct v4l2_rect *b)
>>> +{
>>> + if (a->left < b->left || a->top < b->top)
>>> + return 0;
>>> + if (a->left + a->width > b->left + b->width)
>>> + return 0;
>>> + if (a->top + a->height > b->top + b->height)
>>> + return 0;
>>> +
>>> + return 1;
>>> +}
>>
>> There are helper functions in include/media/v4l2-rect.h, it would make
>> sense to add this one to that header.
>
> I'll check that out.
>
>>
>>> +
>>> +static int vip_s_selection(struct file *file, void *fh,
>>> + struct v4l2_selection *s)
>>> +{
>>> + struct vip_stream *stream = file2stream(file);
>>> + struct v4l2_rect r = s->r;
>>> +
>>> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>> + return -EINVAL;
>>> +
>>> + switch (s->target) {
>>> + case V4L2_SEL_TGT_COMPOSE:
>>> + case V4L2_SEL_TGT_CROP:
>>
>> Why both crop and compose when it is the same c_rect? That makes no sense.
>
> Yeah, this is always puzzling to me. When to use which and what not.
> I'll catch you on IRC sometime to chat about this.
For video capture devices cropping occurs before the DMA step, usually
in the sensor. Composing affects the DMA engine: it composes the (possibly
cropped) video frame into a buffer. E.g. the buffer might be sized for
1920x1080, but the video is 1280x720 and you want to have it DMAed to the
center of the 1920x1080-sized buffer.
This requires that the DMA engine supports this, which is uncommon for video
capture drivers. So typically video capture drivers just support cropping.
>
>>
>>> + v4l_bound_align_image(&r.width, 0, stream->width, 0,
>>> + &r.height, 0, stream->height, 0, 0);
>>> +
>>> + r.left = clamp_t(unsigned int, r.left, 0,
>>> + stream->width - r.width);
>>> + r.top = clamp_t(unsigned int, r.top, 0,
>>> + stream->height - r.height);
>>> +
>>> + if (s->flags & V4L2_SEL_FLAG_LE &&
>>> + !enclosed_rectangle(&r, &s->r))
>>> + return -ERANGE;
>>> +
>>> + if (s->flags & V4L2_SEL_FLAG_GE &&
>>> + !enclosed_rectangle(&s->r, &r))
>>> + return -ERANGE;
>>> +
>>> + s->r = r;
>>> + stream->port->c_rect = r;
>>> +
>>> + vip_dbg(1, stream, "cropped (%d,%d)/%dx%d of %dx%d\n",
>>> + r.left, r.top, r.width, r.height,
>>> + stream->width, stream->height);
>>> +
>>> + s->r = stream->port->c_rect;
>>> + return 0;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + return 0;
>>> +}
Regards,
Hans
^ permalink raw reply
* Re: [PATCHv6 0/4] n_gsm serdev support and protocol driver for droid4 modem
From: Johan Hovold @ 2020-05-28 8:24 UTC (permalink / raw)
To: Tony Lindgren
Cc: Johan Hovold, Greg Kroah-Hartman, Rob Herring, Alan Cox,
Lee Jones, Jiri Slaby, Merlijn Wajer, Pavel Machek, Peter Hurley,
Sebastian Reichel, linux-serial, devicetree, linux-kernel,
linux-omap
In-Reply-To: <20200423153756.GE37466@atomide.com>
Hi Tony,
Sorry about the late reply on this.
On Thu, Apr 23, 2020 at 08:37:56AM -0700, Tony Lindgren wrote:
> * Johan Hovold <johan@kernel.org> [200423 11:44]:
> > I know the location of this driver has been up for discussion already,
> > but drivers/tty/serdev/protocol still isn't right (e.g. we don't have an
> > drivers/i2c/protocol directory where we stuff random i2c client
> > drivers).
>
> Argh, the location of driver again.. So we do have the custom motorola
> layer to deal with on top of TS 27.010, but the custom handling is
> contained within the driver. So maybe just drivers/serial for the
> custom driver then.
Yeah, that should do for now; n_gsm is a serial driver (exposing tty
devices) after all.
> > Last, it seems you've based the serdev-ngsm-motmdm.c chardev
> > implementation on a more or less verbatim copy of drivers/gnss/core.c.
> > I'd appreciate if you could mention that in the file header and
> > reproduce the copyright notice if you end up keeping that interface.
>
> Oh yes indeed, thanks for pointing that out. I'll add it to the next
> version. The chardev code is for sure based on drivers/gnss.
>
> To explain my ignorance, I added the chardev support initially as an
> experiment to see if I can handle the motorola packet layer better
> that way compared to the n_gsm ttys and userspace handling. It ended
> up working quite nicely, so I kept it but then I accidentally left
> out references to the source. Sorry about that.
No worries.
Johan
^ permalink raw reply
* Re: [PATCH RESEND v3 0/2] syscon: Alter syscon and reboot drivers
From: Serge Semin @ 2020-05-28 8:31 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Serge Semin, Alexey Malahov, Maxim Kaurkin, Pavel Parkhomenko,
Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, linux-pm, linux-kernel
In-Reply-To: <20200528070311.uj6bxlplxe2bths5@earth.universe>
On Thu, May 28, 2020 at 09:03:11AM +0200, Sebastian Reichel wrote:
> Hi,
>
> On Tue, May 26, 2020 at 04:50:59PM +0300, Serge Semin wrote:
> > This is a small patchset about tuning the syscon infrastructure a bit.
> > As it's going to be general in the framework of the Baikal-T1 SoC support
> > integration into the kernel, we suggest to replace the legacy text-based
> > syscon-reboot-mode dts-bindings file with yaml-based one. Then seeing a
> > syscon reboot block is normally expected to be a part of a system
> > controller and based on the discussion
> > https://lore.kernel.org/linux-pm/20200306130402.1F4F0803079F@mail.baikalelectronics.ru/
> > we decided to alter the syscon reboot driver so one would also try to fetch
> > the syscon registers map from a parental DT node. regmap property is left
> > supported although it's marked as deprecated from now.
> >
> > This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
> > 0e698dfa2822 ("Linux 5.7-rc4")
> > tag: v5.7-rc4
> >
> > Changelog v2:
> > - Add Sebastian' Acked-by tag to patch 1.
> > - Use a shorter summary describing the bindings modification patches.
> > - Our corporate email server doesn't change Message-Id anymore, so the patchset
> > is resubmitted being in the cover-letter-threaded format.
> > - Discard patch with syscon "-endian" property support. As Rob said It shall be
> > in the common dt-schema.
> > - Replace patches of adding a regmap property support to the syscon-reboot-mode
> > with patches making syscon-reboot a sub-node of a system controller node.
> > - Mark regmap property as deprecated from now.
> >
> > Link: https://lore.kernel.org/linux-pm/20200507233846.11548-1-Sergey.Semin@baikalelectronics.ru/
> > Changelog v3:
> > - Discard the commit 6acd3ecd88ff ("dt-bindings: power: reset: Convert
> > syscon-reboot-mode to DT schema") since it has been merged in by Sebatian.
> > - Add Rob's Reviewed-by tag to the patch "dt-bindings: power: reset: Unrequire
> > regmap property in syscon-reboot node"
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
> > Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
> > Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
> > Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
> > Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
> > Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> >
> > Serge Semin (2):
> > dt-bindings: power: reset: Unrequire regmap property in syscon-reboot
> > node
> > power: reset: syscon-reboot: Add parental syscon support
> >
> > .../bindings/power/reset/syscon-reboot.yaml | 15 ++++++++++-----
> > drivers/power/reset/syscon-reboot.c | 7 +++++--
> > 2 files changed, 15 insertions(+), 7 deletions(-)
>
> Thanks, I queued both patches to power-supply's for-next branch.
>
> -- Sebastian
Great! Thanks.
-Sergey
^ permalink raw reply
* [v1] drm/msm/dpu: allow initialization of encoder locks during encoder init
From: Krishna Manikandan @ 2020-05-28 8:34 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: Krishna Manikandan, linux-kernel, robdclark, seanpaul, hoegsberg,
kalyan_t, nganji, mka
In the current implementation, mutex initialization
for encoder mutex locks are done during encoder
setup. This can lead to scenarios where the lock
is used before it is initialized. Move mutex_init
to dpu_encoder_init to avoid this.
Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f8ac3bf..21730a5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2145,7 +2145,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
dpu_enc = to_dpu_encoder_virt(enc);
- mutex_init(&dpu_enc->enc_lock);
ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info);
if (ret)
goto fail;
@@ -2160,7 +2159,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
0);
- mutex_init(&dpu_enc->rc_lock);
INIT_DELAYED_WORK(&dpu_enc->delayed_off_work,
dpu_encoder_off_work);
dpu_enc->idle_timeout = IDLE_TIMEOUT;
@@ -2205,6 +2203,8 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
spin_lock_init(&dpu_enc->enc_spinlock);
dpu_enc->enabled = false;
+ mutex_init(&dpu_enc->enc_lock);
+ mutex_init(&dpu_enc->rc_lock);
return &dpu_enc->base;
}
--
1.9.1
^ permalink raw reply related
* [v1] drm/msm: add shutdown support for display platform_driver
From: Krishna Manikandan @ 2020-05-28 8:38 UTC (permalink / raw)
To: dri-devel, linux-arm-msm, freedreno, devicetree
Cc: Krishna Manikandan, linux-kernel, robdclark, seanpaul, hoegsberg,
kalyan_t, nganji, mka
Define shutdown callback for display drm driver,
so as to disable all the CRTCS when shutdown
notification is received by the driver.
Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>
---
drivers/gpu/drm/msm/msm_drv.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e4b750b..7a8953f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1322,6 +1322,18 @@ static int msm_pdev_remove(struct platform_device *pdev)
return 0;
}
+static void msm_pdev_shutdown(struct platform_device *pdev)
+{
+ struct drm_device *drm = platform_get_drvdata(pdev);
+
+ if (!drm) {
+ DRM_ERROR("Invalid drm device node\n");
+ return;
+ }
+
+ drm_atomic_helper_shutdown(drm);
+}
+
static const struct of_device_id dt_match[] = {
{ .compatible = "qcom,mdp4", .data = (void *)KMS_MDP4 },
{ .compatible = "qcom,mdss", .data = (void *)KMS_MDP5 },
@@ -1334,6 +1346,7 @@ static int msm_pdev_remove(struct platform_device *pdev)
static struct platform_driver msm_platform_driver = {
.probe = msm_pdev_probe,
.remove = msm_pdev_remove,
+ .shutdown = msm_pdev_shutdown,
.driver = {
.name = "msm",
.of_match_table = dt_match,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 02/11] dt-bindings: i2c: Discard i2c-slave flag from the DW I2C example
From: Serge Semin @ 2020-05-28 8:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Rob Herring, Jarkko Nikula, Wolfram Sang,
Alexey Malahov, Thomas Bogendoerfer, Mika Westerberg, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527175624.GT1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 08:56:24PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 08:18:41PM +0300, Serge Semin wrote:
> > On Wed, May 27, 2020 at 11:12:04AM -0600, Rob Herring wrote:
> > > On Wed, May 27, 2020 at 03:01:02PM +0300, Serge Semin wrote:
> > > > dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
> > > > i2c "reg" property. If it is the compiler will print a warning:
> > > >
> > > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
> > > > Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
> > > >
> > > > In order to silence dtc up let's discard the flag from the DW I2C DT
> > > > binding example for now. Just revert this commit when dtc is fixed.
>
> > > > eeprom@64 {
> > > > compatible = "linux,slave-24c02";
> > > > - reg = <0x40000064>;
> > > > + reg = <0x64>;
> > >
> > > But the compatible is a slave, so you need an example with a different
> > > device.
> >
>
> > Ok. I'll replace the sub-node with just "atmel,24c02" compatible string then.
>
> But how it will be different to the another slave connected to the master?
>
> This example is specifically to show that DesingWare I²C controller may be
> switched to slave mode.
Well, dtc doesn't support it and prints warning that the address is invalid.
Though I do understand you concern and is mostly agree with it. Let's do this in
the next way. I'll resend the series with eeprom@64 sub-node replaced with just
a normal eeprom-device. The message log will have an info why this has been
done. In the non-mergeable section of the patch I'll suggest to Rob reconsider
the patch acking, since we can leave the slave-marked sub-node and just live
with the dtc warning until it's fixed in there.
-Sergey
>
> > > > };
> > > > };
> > > > - |
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCHv8 0/6] n_gsm serdev support and GNSS driver for droid4
From: Johan Hovold @ 2020-05-28 8:39 UTC (permalink / raw)
To: Tony Lindgren
Cc: Greg Kroah-Hartman, Johan Hovold, Rob Herring, Alan Cox,
Lee Jones, Jiri Slaby, Merlijn Wajer, Pavel Machek, Peter Hurley,
Sebastian Reichel, linux-serial, devicetree, linux-kernel,
linux-omap
In-Reply-To: <20200512214713.40501-1-tony@atomide.com>
On Tue, May 12, 2020 at 02:47:07PM -0700, Tony Lindgren wrote:
> Hi all,
>
> Here's the updated set of these patches fixed up for Johan's and
> Pavel's earlier comments.
>
> This series does the following:
>
> 1. Adds functions to n_gsm.c for serdev-ngsm.c driver to use
>
> 2. Adds a generic serdev-ngsm.c driver that brings up the TS 27.010
> TTY ports configured in devicetree with help of n_gsm.c
>
> 3. Allows the use of standard Linux device drivers for dedicated
> TS 27.010 channels for devices like GNSS and ALSA found on some
> modems for example
Unfortunately that does not seem to be the case just yet. Your gnss
driver is still aware that it's using n_gsm for the transport and calls
into the "parent" serdev-ngsm driver instead of using the serdev
interface (e.g. as if this was still and MFD driver).
If you model this right, the GNSS driver should work equally well
regardless of whether you use the serial interface (with n_gsm) or USB
(e.g. cdc-acm or usb-serial).
> 4. Adds a gnss-motmdm consumer driver for the GNSS device found on
> the Motorola Mapphone MDM6600 modem on devices like droid4
>
> I've placed the serdev-ngsm.c under drivers/tty/serdev as it still
> seems to make most sense with no better places available. It's no
> longer an MFD driver as it really does not need to care what channel
> specific consumer drivers might be configured for the generic driver.
> Now serdev-ngsm just uses of_platform_populate() to probe whatever
> child nodes it might find.
>
> I'm not attached having the driver in drivers/tty/serdev. I just
> don't have any better locations in mind. So using Johan's earlier
> i2c example, the drivers/tty/serdev/serdev-ngsm.c driver is now a
> generic protocol and bus driver, so it's getting closer to the
> the drivers/i2c/busses analogy maybe :) Please do suggest better
> locations other than MFD and misc if you have better ideas.
Please move it up one level to drivers/tty where the n_gsm line
discipline lives. This is (supposed to be) a tty driver exposing tty
devices.
> Now without the chardev support, the /dev/gsmtty* using apps need
> to use "U1234AT+CFUN?" format for the packets. The advantage is
> less kernel code, and we keep the existing /dev/gsmtty* interface.
Would it not be possible to deal with this in a plugin sort of way,
again, similar to how the hci ldisc work with an additional "protocol"
ioctl?
Johan
^ permalink raw reply
* Re: [v1] drm/msm: add shutdown support for display platform_driver
From: Sai Prakash Ranjan @ 2020-05-28 8:58 UTC (permalink / raw)
To: Krishna Manikandan
Cc: dri-devel, linux-arm-msm, freedreno, devicetree, linux-kernel,
robdclark, seanpaul, hoegsberg, kalyan_t, nganji, mka,
devicetree-owner, robin.murphy
In-Reply-To: <1590655103-21568-1-git-send-email-mkrishn@codeaurora.org>
Hi Krishna,
On 2020-05-28 14:08, Krishna Manikandan wrote:
> Define shutdown callback for display drm driver,
> so as to disable all the CRTCS when shutdown
> notification is received by the driver.
>
Would be nice to add some more context for adding this shutdown callback
something like below:
If the hardware is still accessing memory after SMMU translation is
disabled
as part of smmu shutdown callback, then the IOVAs(I/O virtual address)
which
it was using will go on the bus as the physical addresses which will
result
in unknown crashes (NoC/interconnect errors on QCOM SoCs).
PS: Credits for description: Robin Murphy
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Sandeep Maheswaram (Temp) @ 2020-05-28 9:24 UTC (permalink / raw)
To: Felipe Balbi, Georgi Djakov, Bjorn Andersson
Cc: Matthias Kaehlcke, Andy Gross, Greg Kroah-Hartman, Rob Herring,
Mark Rutland, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam,
Chandana Kishori Chiluveru, Viresh Kumar
In-Reply-To: <871rn63orz.fsf@kernel.org>
On 5/26/2020 5:13 PM, Felipe Balbi wrote:
> Hi,
>
> Georgi Djakov <georgi.djakov@linaro.org> writes:
>> On 26.05.20 14:04, Sandeep Maheswaram (Temp) wrote:
>>> Hi Felipe,
>>>
>>> Please let me know how to go forward with this patch
> (don't top-post!)
>
>> Please just add a patch to fix the allmodconfig error. Felipe has
>> suggested to introduce a separate patch which exports the
>> device_is_bound() function. This export should precede the addition
>> of interconnect support.
>>
>> Also regarding the "depends on INTERCONNECT || !INTERCONNECT" change,
>> no "depends on" would be needed, as we just made the interconnect
>> framework bool.
> y'all have lost the current merge window, I guess. I'm not sure Greg
> will take last minute changes to drivers base and I have already sent
> him my pull request for v5.8. On the plus side, this gives you the
> chance to run hundreds of randbuilds with your patches.
Hi Georgi,
I am assuming that the patch which exports the device_is_bound() function will solve the allmodconfig error.
Or do i need to change anything in dwc3 driver?
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 1/6] tty: n_gsm: Add support for serdev drivers
From: Johan Hovold @ 2020-05-28 9:31 UTC (permalink / raw)
To: Tony Lindgren
Cc: Greg Kroah-Hartman, Johan Hovold, Rob Herring, Alan Cox,
Lee Jones, Jiri Slaby, Merlijn Wajer, Pavel Machek, Peter Hurley,
Sebastian Reichel, linux-serial, devicetree, linux-kernel,
linux-omap
In-Reply-To: <20200512214713.40501-2-tony@atomide.com>
On Tue, May 12, 2020 at 02:47:08PM -0700, Tony Lindgren wrote:
> We can make use of serdev drivers to do simple device drivers for
> TS 27.010 chanels, and we can handle vendor specific protocols on top
> of TS 27.010 with serdev drivers.
>
> So far this has been tested with Motorola droid4 where there is a custom
> packet numbering protocol on top of TS 27.010 for the MDM6600 modem.
>
> I initially though about adding the serdev support into a separate file,
> but that will take some refactoring of n_gsm.c. And I'd like to have
> things working first. Then later on we might want to consider splitting
> n_gsm.c into three pieces for core, tty and serdev parts. And then maybe
> the serdev related parts can be just moved to live under something like
> drivers/tty/serdev/protocol/ngsm.c.
Yeah, perhaps see where this lands first, but it should probably be done
before merging anything.
And it doesn't really make sense exporting these interfaces without the
actual serdev driver as they are closely tied and cannot be reviewed
separately anyway.
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/tty/n_gsm.c | 435 +++++++++++++++++++++++++++++++++++++
> include/linux/serdev-gsm.h | 154 +++++++++++++
> 2 files changed, 589 insertions(+)
> create mode 100644 include/linux/serdev-gsm.h
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -39,6 +39,7 @@
> #include <linux/file.h>
> #include <linux/uaccess.h>
> #include <linux/module.h>
> +#include <linux/serdev.h>
> #include <linux/timer.h>
> #include <linux/tty_flip.h>
> #include <linux/tty_driver.h>
> @@ -50,6 +51,7 @@
> #include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <linux/gsmmux.h>
> +#include <linux/serdev-gsm.h>
>
> static int debug;
> module_param(debug, int, 0600);
> @@ -150,6 +152,7 @@ struct gsm_dlci {
> /* Data handling callback */
> void (*data)(struct gsm_dlci *dlci, const u8 *data, int len);
> void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len);
> + struct gsm_serdev_dlci *ops; /* serdev dlci ops, if used */
Please rename the struct with a "_operations" suffix as you refer to
this as "ops" throughout.
> struct net_device *net; /* network interface, if created */
> };
>
> @@ -198,6 +201,7 @@ enum gsm_mux_state {
> */
>
> struct gsm_mux {
> + struct gsm_serdev *gsd; /* Serial device bus data */
> struct tty_struct *tty; /* The tty our ldisc is bound to */
> spinlock_t lock;
> struct mutex mutex;
> @@ -2346,6 +2350,437 @@ static int gsm_config(struct gsm_mux *gsm, struct gsm_config *c)
> return 0;
> }
>
> +#ifdef CONFIG_SERIAL_DEV_BUS
> +
> +/**
> + * gsm_serdev_get_config - read ts 27.010 config
> + * @gsd: serdev-gsm instance
> + * @c: ts 27.010 config data
> + *
> + * See gsm_copy_config_values() for more information.
Please document this properly since you're exporting these interfaces.
> + */
> +int gsm_serdev_get_config(struct gsm_serdev *gsd, struct gsm_config *c)
> +{
> + struct gsm_mux *gsm;
> +
> + if (!gsd || !gsd->gsm)
> + return -ENODEV;
Is all this paranoia needed?
> +
> + gsm = gsd->gsm;
> +
> + if (!c)
> + return -EINVAL;
> +
> + gsm_copy_config_values(gsm, c);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_get_config);
> +
> +/**
> + * gsm_serdev_set_config - set ts 27.010 config
> + * @gsd: serdev-gsm instance
> + * @c: ts 27.010 config data
> + *
> + * See gsm_config() for more information.
> + */
> +int gsm_serdev_set_config(struct gsm_serdev *gsd, struct gsm_config *c)
> +{
> + struct gsm_mux *gsm;
> +
> + if (!gsd || !gsd->serdev || !gsd->gsm)
> + return -ENODEV;
And why check for serdev here?
> +
> + gsm = gsd->gsm;
> +
> + if (!c)
> + return -EINVAL;
> +
> + return gsm_config(gsm, c);
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_set_config);
> +
> +static struct gsm_dlci *gsd_dlci_get(struct gsm_serdev *gsd, int line,
> + bool allocate)
> +{
> + struct gsm_mux *gsm;
> + struct gsm_dlci *dlci;
> +
> + if (!gsd || !gsd->gsm)
> + return ERR_PTR(-ENODEV);
> +
> + gsm = gsd->gsm;
> +
> + if (line < 1 || line >= 63)
Line 62 is reserved as well.
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(&gsm->mutex);
> +
> + if (gsm->dlci[line]) {
> + dlci = gsm->dlci[line];
> + goto unlock;
> + } else if (!allocate) {
> + dlci = ERR_PTR(-ENODEV);
> + goto unlock;
> + }
> +
> + dlci = gsm_dlci_alloc(gsm, line);
> + if (!dlci) {
> + dlci = ERR_PTR(-ENOMEM);
> + goto unlock;
> + }
> +
> + gsm->dlci[line] = dlci;
> +
> +unlock:
> + mutex_unlock(&gsm->mutex);
> +
> + return dlci;
> +}
> +
> +static int gsd_dlci_receive_buf(struct gsm_serdev_dlci *ops,
> + const unsigned char *buf,
> + size_t len)
> +{
> + struct gsm_serdev *gsd = ops->gsd;
This looks backwards, why not pass in gsd instead?
> + struct gsm_dlci *dlci;
> + struct tty_port *port;
> +
> + dlci = gsd_dlci_get(gsd, ops->line, false);
> + if (IS_ERR(dlci))
> + return PTR_ERR(dlci);
> +
> + port = &dlci->port;
> + tty_insert_flip_string(port, buf, len);
> + tty_flip_buffer_push(port);
> +
> + return len;
> +}
> +
> +static void gsd_dlci_data(struct gsm_dlci *dlci, const u8 *buf, int len)
> +{
> + struct gsm_mux *gsm = dlci->gsm;
> + struct gsm_serdev *gsd = gsm->gsd;
> +
> + if (!gsd || !dlci->ops)
> + return;
> +
> + switch (dlci->adaption) {
> + case 0:
0 isn't valid, right?
> + case 1:
> + if (dlci->ops->receive_buf)
> + dlci->ops->receive_buf(dlci->ops, buf, len);
> + break;
What about adaption 2 with modem status? Why are you not reusing
gsm_dlci_data()?
> + default:
> + pr_warn("dlci%i adaption %i not yet implemented\n",
> + dlci->addr, dlci->adaption);
This needs to be rate limited. Use the dev_ versions when you can.
> + break;
> + }
> +}
> +
> +/**
> + * gsm_serdev_write - write data to a ts 27.010 channel
> + * @gsd: serdev-gsm instance
> + * @ops: channel ops
> + * @buf: write buffer
> + * @len: buffer length
> + */
> +int gsm_serdev_write(struct gsm_serdev *gsd, struct gsm_serdev_dlci *ops,
> + const u8 *buf, int len)
> +{
> + struct gsm_mux *gsm;
> + struct gsm_dlci *dlci;
> + struct gsm_msg *msg;
> + int h, size, total_size = 0;
> + u8 *dp;
> +
> + if (!gsd || !gsd->gsm)
> + return -ENODEV;
> +
> + gsm = gsd->gsm;
> +
> + dlci = gsd_dlci_get(gsd, ops->line, false);
> + if (IS_ERR(dlci))
> + return PTR_ERR(dlci);
> +
> + h = dlci->adaption - 1;
> +
> + if (len > gsm->mtu)
> + len = gsm->mtu;
> +
> + size = len + h;
> +
> + msg = gsm_data_alloc(gsm, dlci->addr, size, gsm->ftype);
> + if (!msg)
> + return -ENOMEM;
> +
> + dp = msg->data;
> + switch (dlci->adaption) {
> + case 1:
> + break;
> + case 2:
> + *dp++ = gsm_encode_modem(dlci);
> + break;
> + }
> + memcpy(dp, buf, len);
> + gsm_data_queue(dlci, msg);
> + total_size += size;
> +
> + return total_size;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_write);
> +
> +/**
> + * gsm_serdev_data_kick - indicate more data can be transmitted
> + * @gsd: serdev-gsm instance
> + *
> + * See gsm_data_kick() for more information.
> + */
> +void gsm_serdev_data_kick(struct gsm_serdev *gsd)
> +{
> + struct gsm_mux *gsm;
> + unsigned long flags;
> +
> + if (!gsd || !gsd->gsm)
> + return;
> +
> + gsm = gsd->gsm;
> +
> + spin_lock_irqsave(&gsm->tx_lock, flags);
> + gsm_data_kick(gsm);
> + spin_unlock_irqrestore(&gsm->tx_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_data_kick);
> +
> +/**
> + * gsm_serdev_register_dlci - register a ts 27.010 channel
> + * @gsd: serdev-gsm instance
> + * @ops: channel ops
> + */
> +int gsm_serdev_register_dlci(struct gsm_serdev *gsd,
> + struct gsm_serdev_dlci *ops)
> +{
> + struct gsm_dlci *dlci;
> + struct gsm_mux *gsm;
> + int retries;
> +
> + if (!gsd || !gsd->gsm || !gsd->serdev)
> + return -ENODEV;
> +
> + gsm = gsd->gsm;
> +
> + if (!ops || !ops->line)
> + return -EINVAL;
> +
> + dlci = gsd_dlci_get(gsd, ops->line, true);
> + if (IS_ERR(dlci))
> + return PTR_ERR(dlci);
> +
> + if (dlci->state == DLCI_OPENING || dlci->state == DLCI_OPEN ||
> + dlci->state == DLCI_CLOSING)
> + return -EBUSY;
> +
> + mutex_lock(&dlci->mutex);
> + ops->gsd = gsd;
> + dlci->ops = ops;
> + dlci->modem_rx = 0;
> + dlci->prev_data = dlci->data;
I think this one is only used when bringing up a network interface.
> + dlci->data = gsd_dlci_data;
> + mutex_unlock(&dlci->mutex);
> +
> + gsm_dlci_begin_open(dlci);
Why is this here? This should be handled when opening the serial device
(i.e. by gsmtty_open()).
> +
> + /*
> + * Allow some time for dlci to move to DLCI_OPEN state. Otherwise
> + * the serdev consumer driver can start sending data too early during
> + * the setup, and the response will be missed by gms_queue() if we
> + * still have DLCI_CLOSED state.
> + */
> + for (retries = 10; retries > 0; retries--) {
> + if (dlci->state == DLCI_OPEN)
> + break;
> + msleep(100);
> + }
What if you time out? This should be handled properly.
> +
> + if (!retries)
> + dev_dbg(&gsd->serdev->dev, "dlci%i not currently active\n",
> + dlci->addr);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_register_dlci);
> +
> +/**
> + * gsm_serdev_unregister_dlci - unregister a ts 27.010 channel
> + * @gsd: serdev-gsm instance
> + * @ops: channel ops
> + */
> +void gsm_serdev_unregister_dlci(struct gsm_serdev *gsd,
> + struct gsm_serdev_dlci *ops)
> +{
> + struct gsm_mux *gsm;
> + struct gsm_dlci *dlci;
> +
> + if (!gsd || !gsd->gsm || !gsd->serdev)
> + return;
> +
> + gsm = gsd->gsm;
> +
> + if (!ops || !ops->line)
> + return;
> +
> + dlci = gsd_dlci_get(gsd, ops->line, false);
> + if (IS_ERR(dlci))
> + return;
> +
> + mutex_lock(&dlci->mutex);
> + gsm_destroy_network(dlci);
> + dlci->data = dlci->prev_data;
> + dlci->ops->gsd = NULL;
> + dlci->ops = NULL;
> + mutex_unlock(&dlci->mutex);
> +
> + gsm_dlci_begin_close(dlci);
This should all be handled when closing the serial device, that is, by
gsmtty_close().
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_unregister_dlci);
> +
> +static int gsm_serdev_output(struct gsm_mux *gsm, u8 *data, int len)
> +{
> + struct serdev_device *serdev = gsm->gsd->serdev;
> +
> + if (gsm->gsd->output)
> + return gsm->gsd->output(gsm->gsd, data, len);
> + else
> + return serdev_device_write_buf(serdev, data, len);
> +}
I guess this is needed to handle the motorola framing, but not easy to
tell currently due to the split.
> +
> +static int gsd_receive_buf(struct serdev_device *serdev, const u8 *data,
> + size_t count)
> +{
> + struct gsm_serdev *gsd = serdev_device_get_drvdata(serdev);
> + struct gsm_mux *gsm;
> + const unsigned char *dp;
> + int i;
> +
> + if (WARN_ON(!gsd))
> + return 0;
Probably better to take the NULL-deref. Can this ever happen?
> +
> + gsm = gsd->gsm;
> +
> + if (debug & 4)
> + print_hex_dump_bytes("gsd_receive_buf: ",
> + DUMP_PREFIX_OFFSET,
> + data, count);
> +
> + for (i = count, dp = data; i; i--, dp++)
> + gsm->receive(gsm, *dp);
> +
> + return count;
> +}
> +
> +static void gsd_write_wakeup(struct serdev_device *serdev)
> +{
> + serdev_device_write_wakeup(serdev);
> +}
> +
> +static struct serdev_device_ops gsd_client_ops = {
> + .receive_buf = gsd_receive_buf,
> + .write_wakeup = gsd_write_wakeup,
> +};
> +
> +int gsm_serdev_register_tty_port(struct gsm_serdev *gsd, int line)
> +{
> + struct gsm_serdev_dlci *ops;
> + unsigned int base;
> + int error;
> +
> + if (line < 1)
> + return -EINVAL;
Upper limit?
> +
> + ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> + if (!ops)
> + return -ENOMEM;
> +
> + ops->line = line;
> + ops->receive_buf = gsd_dlci_receive_buf;
> +
> + error = gsm_serdev_register_dlci(gsd, ops);
> + if (error) {
> + kfree(ops);
> +
> + return error;
> + }
> +
> + base = mux_num_to_base(gsd->gsm);
> + tty_register_device(gsm_tty_driver, base + ops->line, NULL);
I would expect this to be tty_port_register_device_serdev() so that
serdev gets initialised properly for any client devices (e.g. gnss).
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_register_tty_port);
> +
> +void gsm_serdev_unregister_tty_port(struct gsm_serdev *gsd, int line)
> +{
> + struct gsm_dlci *dlci;
> + unsigned int base;
> +
> + if (line < 1)
> + return;
As above.
> +
> + dlci = gsd_dlci_get(gsd, line, false);
> + if (IS_ERR(dlci))
> + return;
> +
> + base = mux_num_to_base(gsd->gsm);
> + tty_unregister_device(gsm_tty_driver, base + line);
> + gsm_serdev_unregister_dlci(gsd, dlci->ops);
> + kfree(dlci->ops);
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_unregister_tty_port);
> +
> +int gsm_serdev_register_device(struct gsm_serdev *gsd)
> +{
> + struct gsm_mux *gsm;
> + int error;
> +
> + if (WARN(!gsd || !gsd->serdev || !gsd->output,
> + "serdev and output must be initialized\n"))
> + return -EINVAL;
Just oops if the driver is buggy and fails to set essential fields.
> +
> + serdev_device_set_client_ops(gsd->serdev, &gsd_client_ops);
> +
> + gsm = gsm_alloc_mux();
> + if (!gsm)
> + return -ENOMEM;
> +
> + gsm->encoding = 1;
> + gsm->tty = NULL;
> + gsm->gsd = gsd;
> + gsm->output = gsm_serdev_output;
> + gsd->gsm = gsm;
> + mux_get(gsd->gsm);
> +
> + error = gsm_activate_mux(gsd->gsm);
> + if (error) {
> + gsm_cleanup_mux(gsd->gsm);
> + mux_put(gsd->gsm);
> +
> + return error;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_register_device);
> +
> +void gsm_serdev_unregister_device(struct gsm_serdev *gsd)
> +{
> + gsm_cleanup_mux(gsd->gsm);
> + mux_put(gsd->gsm);
> + gsd->gsm = NULL;
> +}
> +EXPORT_SYMBOL_GPL(gsm_serdev_unregister_device);
> +
> +#endif /* CONFIG_SERIAL_DEV_BUS */
It looks like you may also have a problem with tty hangups, which serdev
does not support currently. There are multiple paths in n_gsm which can
trigger a hangup (e.g. based on remote input) and would likely lead to a
crash.
Johan
^ permalink raw reply
* [PATCH v6 01/11] dt-bindings: i2c: Convert DW I2C binding to DT schema
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Rob Herring
Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
Thomas Bogendoerfer, Andy Shevchenko, Mika Westerberg, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces Synopsys DW I2C
legacy bare text bindings with YAML file. As before the bindings file
states that the corresponding dts node is supposed to be compatible
either with generic DW I2C controller or with Microsemi Ocelot SoC I2C
one, to have registers, interrupts and clocks properties. In addition
the node may have clock-frequency, i2c-sda-hold-time-ns,
i2c-scl-falling-time-ns and i2c-sda-falling-time-ns optional properties.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Make sure that "mscc,ocelot-i2c" compatible node may have up to two
registers space defined, while normal DW I2C controller will have only
one registers space.
- Add "mscc,ocelot-i2c" example to test the previous fix.
- Declare "unevaluatedProperties" property instead of
"additionalProperties" one.
- Due to the previous fix we can now discard the dummy boolean properties
definitions, since the proper type evaluation will be performed by the
generic i2c-controller.yaml schema.
Changelog v3:
- Discard $ref from the "-ns" suffixed properties since they've got the
uint32-array type by default applied in the common schema. Set "maxItems: 1"
there instead to make sure the property will have a single value specified.
---
.../bindings/i2c/i2c-designware.txt | 73 ---------
.../bindings/i2c/snps,designware-i2c.yaml | 154 ++++++++++++++++++
2 files changed, 154 insertions(+), 73 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
deleted file mode 100644
index 08be4d3846e5..000000000000
--- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
+++ /dev/null
@@ -1,73 +0,0 @@
-* Synopsys DesignWare I2C
-
-Required properties :
-
- - compatible : should be "snps,designware-i2c"
- or "mscc,ocelot-i2c" with "snps,designware-i2c" for fallback
- - reg : Offset and length of the register set for the device
- - interrupts : <IRQ> where IRQ is the interrupt number.
- - clocks : phandles for the clocks, see the description of clock-names below.
- The phandle for the "ic_clk" clock is required. The phandle for the "pclk"
- clock is optional. If a single clock is specified but no clock-name, it is
- the "ic_clk" clock. If both clocks are listed, the "ic_clk" must be first.
-
-Recommended properties :
-
- - clock-frequency : desired I2C bus clock frequency in Hz.
-
-Optional properties :
-
- - clock-names : Contains the names of the clocks:
- "ic_clk", for the core clock used to generate the external I2C clock.
- "pclk", the interface clock, required for register access.
-
- - reg : for "mscc,ocelot-i2c", a second register set to configure the SDA hold
- time, named ICPU_CFG:TWI_DELAY in the datasheet.
-
- - i2c-sda-hold-time-ns : should contain the SDA hold time in nanoseconds.
- This option is only supported in hardware blocks version 1.11a or newer and
- on Microsemi SoCs ("mscc,ocelot-i2c" compatible).
-
- - i2c-scl-falling-time-ns : should contain the SCL falling time in nanoseconds.
- This value which is by default 300ns is used to compute the tLOW period.
-
- - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
- This value which is by default 300ns is used to compute the tHIGH period.
-
-Examples :
-
- i2c@f0000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "snps,designware-i2c";
- reg = <0xf0000 0x1000>;
- interrupts = <11>;
- clock-frequency = <400000>;
- };
-
- i2c@1120000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "snps,designware-i2c";
- reg = <0x1120000 0x1000>;
- interrupt-parent = <&ictl>;
- interrupts = <12 1>;
- clock-frequency = <400000>;
- i2c-sda-hold-time-ns = <300>;
- i2c-sda-falling-time-ns = <300>;
- i2c-scl-falling-time-ns = <300>;
- };
-
- i2c@1120000 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <0x2000 0x100>;
- clock-frequency = <400000>;
- clocks = <&i2cclk>;
- interrupts = <0>;
-
- eeprom@64 {
- compatible = "linux,slave-24c02";
- reg = <0x40000064>;
- };
- };
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
new file mode 100644
index 000000000000..4bd430b2b41d
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/snps,designware-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB I2C Controller
+
+maintainers:
+ - Jarkko Nikula <jarkko.nikula@linux.intel.com>
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ not:
+ contains:
+ const: mscc,ocelot-i2c
+ then:
+ properties:
+ reg:
+ maxItems: 1
+
+properties:
+ compatible:
+ oneOf:
+ - description: Generic Synopsys DesignWare I2C controller
+ const: snps,designware-i2c
+ - description: Microsemi Ocelot SoCs I2C controller
+ items:
+ - const: mscc,ocelot-i2c
+ - const: snps,designware-i2c
+
+ reg:
+ minItems: 1
+ items:
+ - description: DW APB I2C controller memory mapped registers
+ - description: |
+ ICPU_CFG:TWI_DELAY registers to setup the SDA hold time.
+ This registers are specific to the Ocelot I2C-controller.
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ items:
+ - description: I2C controller reference clock source
+ - description: APB interface clock source
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: ref
+ - const: pclk
+
+ resets:
+ maxItems: 1
+
+ clock-frequency:
+ description: Desired I2C bus clock frequency in Hz
+ enum: [100000, 400000, 1000000, 3400000]
+ default: 400000
+
+ i2c-sda-hold-time-ns:
+ maxItems: 1
+ description: |
+ The property should contain the SDA hold time in nanoseconds. This option
+ is only supported in hardware blocks version 1.11a or newer or on
+ Microsemi SoCs.
+
+ i2c-scl-falling-time-ns:
+ maxItems: 1
+ description: |
+ The property should contain the SCL falling time in nanoseconds.
+ This value is used to compute the tLOW period.
+ default: 300
+
+ i2c-sda-falling-time-ns:
+ maxItems: 1
+ description: |
+ The property should contain the SDA falling time in nanoseconds.
+ This value is used to compute the tHIGH period.
+ default: 300
+
+ dmas:
+ items:
+ - description: TX DMA Channel
+ - description: RX DMA Channel
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - interrupts
+
+examples:
+ - |
+ i2c@f0000 {
+ compatible = "snps,designware-i2c";
+ reg = <0xf0000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <11>;
+ clock-frequency = <400000>;
+ };
+ - |
+ i2c@1120000 {
+ compatible = "snps,designware-i2c";
+ reg = <0x1120000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <12 1>;
+ clock-frequency = <400000>;
+ i2c-sda-hold-time-ns = <300>;
+ i2c-sda-falling-time-ns = <300>;
+ i2c-scl-falling-time-ns = <300>;
+ };
+ - |
+ i2c@2000 {
+ compatible = "snps,designware-i2c";
+ reg = <0x2000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clock-frequency = <400000>;
+ clocks = <&i2cclk>;
+ interrupts = <0>;
+
+ eeprom@64 {
+ compatible = "linux,slave-24c02";
+ reg = <0x40000064>;
+ };
+ };
+ - |
+ i2c@100400 {
+ compatible = "mscc,ocelot-i2c", "snps,designware-i2c";
+ reg = <0x100400 0x100>, <0x198 0x8>;
+ pinctrl-0 = <&i2c_pins>;
+ pinctrl-names = "default";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <8>;
+ clocks = <&ahb_clk>;
+ };
+...
--
2.26.2
^ permalink raw reply related
* [PATCH v6 08/11] i2c: designware: Convert driver to using regmap API
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Rob Herring, devicetree, linux-mips, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Seeing the DW I2C driver is using flags-based accessors with two
conditional clauses it would be better to replace them with the regmap
API IO methods and to initialize the regmap object with read/write
callbacks specific to the controller registers map implementation. This
will be also handy for the drivers with non-standard registers mapping
(like an embedded into the Baikal-T1 System Controller DW I2C block, which
glue-driver is a part of this series).
As before the driver tries to detect the mapping setup at probe stage and
creates a regmap object accordingly, which will be used by the rest of the
code to correctly access the controller registers. In two places it was
appropriate to convert the hand-written read-modify-write and
read-poll-loop design patterns to the corresponding regmap API
ready-to-use methods.
Note the regmap IO methods return value is checked only at the probe
stage. The rest of the code won't do this because basically we have
MMIO-based regmap so non of the read/write methods can fail (this also
won't be needed for the Baikal-T1-specific I2C controller).
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-mips@vger.kernel.org
---
Changelog v5:
- Keep alphabetical order of the include statements.
- Discard explicit u16-type cast in the dw_reg_write_word() method.
Changelog v6:
- Add comma in the last explicitly initialized member of the map_cfg
struct regmap_config instance.
---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-designware-common.c | 178 +++++++++++++++------
drivers/i2c/busses/i2c-designware-core.h | 22 ++-
drivers/i2c/busses/i2c-designware-master.c | 125 ++++++++-------
drivers/i2c/busses/i2c-designware-slave.c | 77 ++++-----
5 files changed, 248 insertions(+), 155 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7cd279c36898..259e2325712a 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -526,6 +526,7 @@ config I2C_DAVINCI
config I2C_DESIGNWARE_CORE
tristate
+ select REGMAP
config I2C_DESIGNWARE_SLAVE
bool "Synopsys DesignWare Slave"
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index ed302342f8db..0b190a3c837c 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -21,6 +21,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/swab.h>
#include <linux/types.h>
@@ -57,66 +58,123 @@ static char *abort_sources[] = {
"incorrect slave-transmitter mode configuration",
};
-u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
{
- u32 value;
+ struct dw_i2c_dev *dev = context;
- if (dev->flags & ACCESS_16BIT)
- value = readw_relaxed(dev->base + offset) |
- (readw_relaxed(dev->base + offset + 2) << 16);
- else
- value = readl_relaxed(dev->base + offset);
+ *val = readl_relaxed(dev->base + reg);
- if (dev->flags & ACCESS_SWAP)
- return swab32(value);
- else
- return value;
+ return 0;
}
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+static int dw_reg_write(void *context, unsigned int reg, unsigned int val)
{
- if (dev->flags & ACCESS_SWAP)
- b = swab32(b);
-
- if (dev->flags & ACCESS_16BIT) {
- writew_relaxed((u16)b, dev->base + offset);
- writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
- } else {
- writel_relaxed(b, dev->base + offset);
- }
+ struct dw_i2c_dev *dev = context;
+
+ writel_relaxed(val, dev->base + reg);
+
+ return 0;
+}
+
+static int dw_reg_read_swab(void *context, unsigned int reg, unsigned int *val)
+{
+ struct dw_i2c_dev *dev = context;
+
+ *val = swab32(readl_relaxed(dev->base + reg));
+
+ return 0;
+}
+
+static int dw_reg_write_swab(void *context, unsigned int reg, unsigned int val)
+{
+ struct dw_i2c_dev *dev = context;
+
+ writel_relaxed(swab32(val), dev->base + reg);
+
+ return 0;
+}
+
+static int dw_reg_read_word(void *context, unsigned int reg, unsigned int *val)
+{
+ struct dw_i2c_dev *dev = context;
+
+ *val = readw_relaxed(dev->base + reg) |
+ (readw_relaxed(dev->base + reg + 2) << 16);
+
+ return 0;
+}
+
+static int dw_reg_write_word(void *context, unsigned int reg, unsigned int val)
+{
+ struct dw_i2c_dev *dev = context;
+
+ writew_relaxed(val, dev->base + reg);
+ writew_relaxed(val >> 16, dev->base + reg + 2);
+
+ return 0;
}
/**
- * i2c_dw_set_reg_access() - Set register access flags
+ * i2c_dw_init_regmap() - Initialize registers map
* @dev: device private data
+ * @base: Registers map base address
*
- * Autodetects needed register access mode and sets access flags accordingly.
- * This must be called before doing any other register access.
+ * Autodetects needed register access mode and creates the regmap with
+ * corresponding read/write callbacks. This must be called before doing any
+ * other register access.
*/
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
{
+ struct regmap_config map_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .disable_locking = true,
+ .reg_read = dw_reg_read,
+ .reg_write = dw_reg_write,
+ .max_register = DW_IC_COMP_TYPE,
+ };
u32 reg;
int ret;
+ /*
+ * Skip detecting the registers map configuration if the regmap has
+ * already been provided by a higher code.
+ */
+ if (dev->map)
+ return 0;
+
ret = i2c_dw_acquire_lock(dev);
if (ret)
return ret;
- reg = dw_readl(dev, DW_IC_COMP_TYPE);
+ reg = readl(dev->base + DW_IC_COMP_TYPE);
i2c_dw_release_lock(dev);
if (reg == swab32(DW_IC_COMP_TYPE_VALUE)) {
- /* Configure register endianness access */
- dev->flags |= ACCESS_SWAP;
+ map_cfg.reg_read = dw_reg_read_swab;
+ map_cfg.reg_write = dw_reg_write_swab;
} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
- /* Configure register access mode 16bit */
- dev->flags |= ACCESS_16BIT;
+ map_cfg.reg_read = dw_reg_read_word;
+ map_cfg.reg_write = dw_reg_write_word;
} else if (reg != DW_IC_COMP_TYPE_VALUE) {
dev_err(dev->dev,
"Unknown Synopsys component type: 0x%08x\n", reg);
return -ENODEV;
}
+ /*
+ * Note we'll check the return value of the regmap IO accessors only
+ * at the probe stage. The rest of the code won't do this because
+ * basically we have MMIO-based regmap so non of the read/write methods
+ * can fail.
+ */
+ dev->map = devm_regmap_init(dev->dev, NULL, dev, &map_cfg);
+ if (IS_ERR(dev->map)) {
+ dev_err(dev->dev, "Failed to init the registers map\n");
+ return PTR_ERR(dev->map);
+ }
+
return 0;
}
@@ -327,11 +385,17 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
return ret;
/* Configure SDA Hold Time if required */
- reg = dw_readl(dev, DW_IC_COMP_VERSION);
+ ret = regmap_read(dev->map, DW_IC_COMP_VERSION, ®);
+ if (ret)
+ goto err_release_lock;
+
if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
if (!dev->sda_hold_time) {
/* Keep previous hold time setting if no one set it */
- dev->sda_hold_time = dw_readl(dev, DW_IC_SDA_HOLD);
+ ret = regmap_read(dev->map, DW_IC_SDA_HOLD,
+ &dev->sda_hold_time);
+ if (ret)
+ goto err_release_lock;
}
/*
@@ -355,14 +419,16 @@ int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev)
dev->sda_hold_time = 0;
}
+err_release_lock:
i2c_dw_release_lock(dev);
- return 0;
+ return ret;
}
void __i2c_dw_disable(struct dw_i2c_dev *dev)
{
int timeout = 100;
+ u32 status;
do {
__i2c_dw_disable_nowait(dev);
@@ -370,7 +436,8 @@ void __i2c_dw_disable(struct dw_i2c_dev *dev)
* The enable status register may be unimplemented, but
* in that case this test reads zero and exits the loop.
*/
- if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == 0)
+ regmap_read(dev->map, DW_IC_ENABLE_STATUS, &status);
+ if ((status & 1) == 0)
return;
/*
@@ -449,22 +516,23 @@ void i2c_dw_release_lock(struct dw_i2c_dev *dev)
*/
int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
{
- int timeout = TIMEOUT;
+ u32 status;
+ int ret;
- while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
- if (timeout <= 0) {
- dev_warn(dev->dev, "timeout waiting for bus ready\n");
- i2c_recover_bus(&dev->adapter);
+ ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+ !(status & DW_IC_STATUS_ACTIVITY),
+ 1100, 20000);
+ if (ret) {
+ dev_warn(dev->dev, "timeout waiting for bus ready\n");
- if (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY)
- return -ETIMEDOUT;
- return 0;
- }
- timeout--;
- usleep_range(1000, 1100);
+ i2c_recover_bus(&dev->adapter);
+
+ regmap_read(dev->map, DW_IC_STATUS, &status);
+ if (!(status & DW_IC_STATUS_ACTIVITY))
+ ret = 0;
}
- return 0;
+ return ret;
}
int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
@@ -490,15 +558,19 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
return -EIO;
}
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
{
u32 param, tx_fifo_depth, rx_fifo_depth;
+ int ret;
/*
* Try to detect the FIFO depth if not set by interface driver,
* the depth could be from 2 to 256 from HW spec.
*/
- param = dw_readl(dev, DW_IC_COMP_PARAM_1);
+ ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, ¶m);
+ if (ret)
+ return ret;
+
tx_fifo_depth = ((param >> 16) & 0xff) + 1;
rx_fifo_depth = ((param >> 8) & 0xff) + 1;
if (!dev->tx_fifo_depth) {
@@ -510,6 +582,8 @@ void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
dev->rx_fifo_depth = min_t(u32, dev->rx_fifo_depth,
rx_fifo_depth);
}
+
+ return 0;
}
u32 i2c_dw_func(struct i2c_adapter *adap)
@@ -521,17 +595,19 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
void i2c_dw_disable(struct dw_i2c_dev *dev)
{
+ u32 dummy;
+
/* Disable controller */
__i2c_dw_disable(dev);
/* Disable all interrupts */
- dw_writel(dev, 0, DW_IC_INTR_MASK);
- dw_readl(dev, DW_IC_CLR_INTR);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+ regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
}
void i2c_dw_disable_int(struct dw_i2c_dev *dev)
{
- dw_writel(dev, 0, DW_IC_INTR_MASK);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
}
MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b9ef9b0deef0..f5bbe3d6bcf8 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -15,6 +15,7 @@
#include <linux/dev_printk.h>
#include <linux/errno.h>
#include <linux/i2c.h>
+#include <linux/regmap.h>
#include <linux/types.h>
#define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
@@ -126,8 +127,6 @@
#define STATUS_WRITE_IN_PROGRESS 0x1
#define STATUS_READ_IN_PROGRESS 0x2
-#define TIMEOUT 20 /* ms */
-
/*
* operation modes
*/
@@ -183,7 +182,9 @@ struct reset_control;
/**
* struct dw_i2c_dev - private i2c-designware data
* @dev: driver model device node
+ * @map: IO registers map
* @base: IO registers pointer
+ * @ext: Extended IO registers pointer
* @cmd_complete: tx completion indicator
* @clk: input reference clock
* @pclk: clock required to access the registers
@@ -233,6 +234,7 @@ struct reset_control;
*/
struct dw_i2c_dev {
struct device *dev;
+ struct regmap *map;
void __iomem *base;
void __iomem *ext;
struct completion cmd_complete;
@@ -284,17 +286,13 @@ struct dw_i2c_dev {
bool suspended;
};
-#define ACCESS_SWAP 0x00000001
-#define ACCESS_16BIT 0x00000002
-#define ACCESS_INTR_MASK 0x00000004
-#define ACCESS_NO_IRQ_SUSPEND 0x00000008
+#define ACCESS_INTR_MASK 0x00000001
+#define ACCESS_NO_IRQ_SUSPEND 0x00000002
#define MODEL_MSCC_OCELOT 0x00000100
#define MODEL_MASK 0x00000f00
-u32 dw_readl(struct dw_i2c_dev *dev, int offset);
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
-int i2c_dw_set_reg_access(struct dw_i2c_dev *dev);
+int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset);
u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset);
int i2c_dw_set_sda_hold(struct dw_i2c_dev *dev);
@@ -304,19 +302,19 @@ int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
void i2c_dw_release_lock(struct dw_i2c_dev *dev);
int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
-void i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
+int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev);
u32 i2c_dw_func(struct i2c_adapter *adap);
void i2c_dw_disable(struct dw_i2c_dev *dev);
void i2c_dw_disable_int(struct dw_i2c_dev *dev);
static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
{
- dw_writel(dev, 1, DW_IC_ENABLE);
+ regmap_write(dev->map, DW_IC_ENABLE, 1);
}
static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
{
- dw_writel(dev, 0, DW_IC_ENABLE);
+ regmap_write(dev->map, DW_IC_ENABLE, 0);
}
void __i2c_dw_disable(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 95eeec53c744..2cba21b945d8 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -18,6 +18,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include "i2c-designware-core.h"
@@ -25,11 +26,11 @@
static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
{
/* Configure Tx/Rx FIFO threshold levels */
- dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
- dw_writel(dev, 0, DW_IC_RX_TL);
+ regmap_write(dev->map, DW_IC_TX_TL, dev->tx_fifo_depth / 2);
+ regmap_write(dev->map, DW_IC_RX_TL, 0);
/* Configure the I2C master */
- dw_writel(dev, dev->master_cfg, DW_IC_CON);
+ regmap_write(dev->map, DW_IC_CON, dev->master_cfg);
}
static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
@@ -44,8 +45,11 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
ret = i2c_dw_acquire_lock(dev);
if (ret)
return ret;
- comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
+
+ ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &comp_param1);
i2c_dw_release_lock(dev);
+ if (ret)
+ return ret;
/* Set standard and fast speed dividers for high/low periods */
sda_falling_time = t->sda_fall_ns ?: 300; /* ns */
@@ -187,22 +191,22 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
__i2c_dw_disable(dev);
/* Write standard speed timing parameters */
- dw_writel(dev, dev->ss_hcnt, DW_IC_SS_SCL_HCNT);
- dw_writel(dev, dev->ss_lcnt, DW_IC_SS_SCL_LCNT);
+ regmap_write(dev->map, DW_IC_SS_SCL_HCNT, dev->ss_hcnt);
+ regmap_write(dev->map, DW_IC_SS_SCL_LCNT, dev->ss_lcnt);
/* Write fast mode/fast mode plus timing parameters */
- dw_writel(dev, dev->fs_hcnt, DW_IC_FS_SCL_HCNT);
- dw_writel(dev, dev->fs_lcnt, DW_IC_FS_SCL_LCNT);
+ regmap_write(dev->map, DW_IC_FS_SCL_HCNT, dev->fs_hcnt);
+ regmap_write(dev->map, DW_IC_FS_SCL_LCNT, dev->fs_lcnt);
/* Write high speed timing parameters if supported */
if (dev->hs_hcnt && dev->hs_lcnt) {
- dw_writel(dev, dev->hs_hcnt, DW_IC_HS_SCL_HCNT);
- dw_writel(dev, dev->hs_lcnt, DW_IC_HS_SCL_LCNT);
+ regmap_write(dev->map, DW_IC_HS_SCL_HCNT, dev->hs_hcnt);
+ regmap_write(dev->map, DW_IC_HS_SCL_LCNT, dev->hs_lcnt);
}
/* Write SDA hold time if supported */
if (dev->sda_hold_time)
- dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
+ regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
i2c_dw_configure_fifo_master(dev);
i2c_dw_release_lock(dev);
@@ -213,15 +217,15 @@ static int i2c_dw_init_master(struct dw_i2c_dev *dev)
static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
- u32 ic_con, ic_tar = 0;
+ u32 ic_con = 0, ic_tar = 0;
+ u32 dummy;
/* Disable the adapter */
__i2c_dw_disable(dev);
/* If the slave address is ten bit address, enable 10BITADDR */
- ic_con = dw_readl(dev, DW_IC_CON);
if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
- ic_con |= DW_IC_CON_10BITADDR_MASTER;
+ ic_con = DW_IC_CON_10BITADDR_MASTER;
/*
* If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
* mode has to be enabled via bit 12 of IC_TAR register.
@@ -229,17 +233,17 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
* detected from registers.
*/
ic_tar = DW_IC_TAR_10BITADDR_MASTER;
- } else {
- ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
}
- dw_writel(dev, ic_con, DW_IC_CON);
+ regmap_update_bits(dev->map, DW_IC_CON, DW_IC_CON_10BITADDR_MASTER,
+ ic_con);
/*
* Set the slave (target) address and enable 10-bit addressing mode
* if applicable.
*/
- dw_writel(dev, msgs[dev->msg_write_idx].addr | ic_tar, DW_IC_TAR);
+ regmap_write(dev->map, DW_IC_TAR,
+ msgs[dev->msg_write_idx].addr | ic_tar);
/* Enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);
@@ -248,11 +252,11 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
__i2c_dw_enable(dev);
/* Dummy read to avoid the register getting stuck on Bay Trail */
- dw_readl(dev, DW_IC_ENABLE_STATUS);
+ regmap_read(dev->map, DW_IC_ENABLE_STATUS, &dummy);
/* Clear and enable interrupts */
- dw_readl(dev, DW_IC_CLR_INTR);
- dw_writel(dev, DW_IC_INTR_MASTER_MASK, DW_IC_INTR_MASK);
+ regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
+ regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_MASTER_MASK);
}
/*
@@ -271,6 +275,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
u32 buf_len = dev->tx_buf_len;
u8 *buf = dev->tx_buf;
bool need_restart = false;
+ unsigned int flr;
intr_mask = DW_IC_INTR_MASTER_MASK;
@@ -303,8 +308,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
need_restart = true;
}
- tx_limit = dev->tx_fifo_depth - dw_readl(dev, DW_IC_TXFLR);
- rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
+ regmap_read(dev->map, DW_IC_TXFLR, &flr);
+ tx_limit = dev->tx_fifo_depth - flr;
+
+ regmap_read(dev->map, DW_IC_RXFLR, &flr);
+ rx_limit = dev->rx_fifo_depth - flr;
while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
u32 cmd = 0;
@@ -337,11 +345,14 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
- dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
+ regmap_write(dev->map, DW_IC_DATA_CMD,
+ cmd | 0x100);
rx_limit--;
dev->rx_outstanding++;
- } else
- dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
+ } else {
+ regmap_write(dev->map, DW_IC_DATA_CMD,
+ cmd | *buf++);
+ }
tx_limit--; buf_len--;
}
@@ -371,7 +382,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (dev->msg_err)
intr_mask = 0;
- dw_writel(dev, intr_mask, DW_IC_INTR_MASK);
+ regmap_write(dev->map, DW_IC_INTR_MASK, intr_mask);
}
static u8
@@ -399,7 +410,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
int rx_valid;
for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
- u32 len;
+ u32 len, tmp;
u8 *buf;
if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
@@ -413,18 +424,18 @@ i2c_dw_read(struct dw_i2c_dev *dev)
buf = dev->rx_buf;
}
- rx_valid = dw_readl(dev, DW_IC_RXFLR);
+ regmap_read(dev->map, DW_IC_RXFLR, &rx_valid);
for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
u32 flags = msgs[dev->msg_read_idx].flags;
- *buf = dw_readl(dev, DW_IC_DATA_CMD);
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
/* Ensure length byte is a valid value */
if (flags & I2C_M_RECV_LEN &&
- *buf <= I2C_SMBUS_BLOCK_MAX && *buf > 0) {
- len = i2c_dw_recv_len(dev, *buf);
+ tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) {
+ len = i2c_dw_recv_len(dev, tmp);
}
- buf++;
+ *buf++ = tmp;
dev->rx_outstanding--;
}
@@ -542,7 +553,7 @@ static const struct i2c_adapter_quirks i2c_dw_quirks = {
static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
{
- u32 stat;
+ u32 stat, dummy;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -550,47 +561,47 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
* in the IC_RAW_INTR_STAT register.
*
* That is,
- * stat = dw_readl(IC_INTR_STAT);
+ * stat = readl(IC_INTR_STAT);
* equals to,
- * stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
+ * stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
*
* The raw version might be useful for debugging purposes.
*/
- stat = dw_readl(dev, DW_IC_INTR_STAT);
+ regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
/*
* Do not use the IC_CLR_INTR register to clear interrupts, or
* you'll miss some interrupts, triggered during the period from
- * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
+ * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
*
* Instead, use the separately-prepared IC_CLR_* registers.
*/
if (stat & DW_IC_INTR_RX_UNDER)
- dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
if (stat & DW_IC_INTR_RX_OVER)
- dw_readl(dev, DW_IC_CLR_RX_OVER);
+ regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
if (stat & DW_IC_INTR_TX_OVER)
- dw_readl(dev, DW_IC_CLR_TX_OVER);
+ regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
if (stat & DW_IC_INTR_RD_REQ)
- dw_readl(dev, DW_IC_CLR_RD_REQ);
+ regmap_read(dev->map, DW_IC_CLR_RD_REQ, &dummy);
if (stat & DW_IC_INTR_TX_ABRT) {
/*
* The IC_TX_ABRT_SOURCE register is cleared whenever
* the IC_CLR_TX_ABRT is read. Preserve it beforehand.
*/
- dev->abort_source = dw_readl(dev, DW_IC_TX_ABRT_SOURCE);
- dw_readl(dev, DW_IC_CLR_TX_ABRT);
+ regmap_read(dev->map, DW_IC_TX_ABRT_SOURCE, &dev->abort_source);
+ regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
}
if (stat & DW_IC_INTR_RX_DONE)
- dw_readl(dev, DW_IC_CLR_RX_DONE);
+ regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
if (stat & DW_IC_INTR_ACTIVITY)
- dw_readl(dev, DW_IC_CLR_ACTIVITY);
+ regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
if (stat & DW_IC_INTR_STOP_DET)
- dw_readl(dev, DW_IC_CLR_STOP_DET);
+ regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
if (stat & DW_IC_INTR_START_DET)
- dw_readl(dev, DW_IC_CLR_START_DET);
+ regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
if (stat & DW_IC_INTR_GEN_CALL)
- dw_readl(dev, DW_IC_CLR_GEN_CALL);
+ regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
return stat;
}
@@ -612,7 +623,7 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
* Anytime TX_ABRT is set, the contents of the tx/rx
* buffers are flushed. Make sure to skip them.
*/
- dw_writel(dev, 0, DW_IC_INTR_MASK);
+ regmap_write(dev->map, DW_IC_INTR_MASK, 0);
goto tx_aborted;
}
@@ -633,9 +644,9 @@ static int i2c_dw_irq_handler_master(struct dw_i2c_dev *dev)
complete(&dev->cmd_complete);
else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
/* Workaround to trigger pending interrupt */
- stat = dw_readl(dev, DW_IC_INTR_MASK);
+ regmap_read(dev->map, DW_IC_INTR_MASK, &stat);
i2c_dw_disable_int(dev);
- dw_writel(dev, stat, DW_IC_INTR_MASK);
+ regmap_write(dev->map, DW_IC_INTR_MASK, stat);
}
return 0;
@@ -646,8 +657,8 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
struct dw_i2c_dev *dev = dev_id;
u32 stat, enabled;
- enabled = dw_readl(dev, DW_IC_ENABLE);
- stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
+ regmap_read(dev->map, DW_IC_ENABLE, &enabled);
+ regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &stat);
dev_dbg(dev->dev, "enabled=%#x stat=%#x\n", enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
@@ -739,7 +750,7 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
dev->disable = i2c_dw_disable;
dev->disable_int = i2c_dw_disable_int;
- ret = i2c_dw_set_reg_access(dev);
+ ret = i2c_dw_init_regmap(dev);
if (ret)
return ret;
@@ -747,7 +758,9 @@ int i2c_dw_probe_master(struct dw_i2c_dev *dev)
if (ret)
return ret;
- i2c_dw_set_fifo_size(dev);
+ ret = i2c_dw_set_fifo_size(dev);
+ if (ret)
+ return ret;
ret = dev->init(dev);
if (ret)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 576e7af4e94b..44974b53a626 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -14,18 +14,19 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include "i2c-designware-core.h"
static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
{
/* Configure Tx/Rx FIFO threshold levels. */
- dw_writel(dev, 0, DW_IC_TX_TL);
- dw_writel(dev, 0, DW_IC_RX_TL);
+ regmap_write(dev->map, DW_IC_TX_TL, 0);
+ regmap_write(dev->map, DW_IC_RX_TL, 0);
/* Configure the I2C slave. */
- dw_writel(dev, dev->slave_cfg, DW_IC_CON);
- dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);
+ regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
+ regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
}
/**
@@ -49,7 +50,7 @@ static int i2c_dw_init_slave(struct dw_i2c_dev *dev)
/* Write SDA hold time if supported */
if (dev->sda_hold_time)
- dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
+ regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
i2c_dw_configure_fifo_slave(dev);
i2c_dw_release_lock(dev);
@@ -72,7 +73,7 @@ static int i2c_dw_reg_slave(struct i2c_client *slave)
* the address to which the DW_apb_i2c responds.
*/
__i2c_dw_disable_nowait(dev);
- dw_writel(dev, slave->addr, DW_IC_SAR);
+ regmap_write(dev->map, DW_IC_SAR, slave->addr);
dev->slave = slave;
__i2c_dw_enable(dev);
@@ -103,7 +104,7 @@ static int i2c_dw_unreg_slave(struct i2c_client *slave)
static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
{
- u32 stat;
+ u32 stat, dummy;
/*
* The IC_INTR_STAT register just indicates "enabled" interrupts.
@@ -111,39 +112,39 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
* in the IC_RAW_INTR_STAT register.
*
* That is,
- * stat = dw_readl(IC_INTR_STAT);
+ * stat = readl(IC_INTR_STAT);
* equals to,
- * stat = dw_readl(IC_RAW_INTR_STAT) & dw_readl(IC_INTR_MASK);
+ * stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
*
* The raw version might be useful for debugging purposes.
*/
- stat = dw_readl(dev, DW_IC_INTR_STAT);
+ regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
/*
* Do not use the IC_CLR_INTR register to clear interrupts, or
* you'll miss some interrupts, triggered during the period from
- * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
+ * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
*
* Instead, use the separately-prepared IC_CLR_* registers.
*/
if (stat & DW_IC_INTR_TX_ABRT)
- dw_readl(dev, DW_IC_CLR_TX_ABRT);
+ regmap_read(dev->map, DW_IC_CLR_TX_ABRT, &dummy);
if (stat & DW_IC_INTR_RX_UNDER)
- dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &dummy);
if (stat & DW_IC_INTR_RX_OVER)
- dw_readl(dev, DW_IC_CLR_RX_OVER);
+ regmap_read(dev->map, DW_IC_CLR_RX_OVER, &dummy);
if (stat & DW_IC_INTR_TX_OVER)
- dw_readl(dev, DW_IC_CLR_TX_OVER);
+ regmap_read(dev->map, DW_IC_CLR_TX_OVER, &dummy);
if (stat & DW_IC_INTR_RX_DONE)
- dw_readl(dev, DW_IC_CLR_RX_DONE);
+ regmap_read(dev->map, DW_IC_CLR_RX_DONE, &dummy);
if (stat & DW_IC_INTR_ACTIVITY)
- dw_readl(dev, DW_IC_CLR_ACTIVITY);
+ regmap_read(dev->map, DW_IC_CLR_ACTIVITY, &dummy);
if (stat & DW_IC_INTR_STOP_DET)
- dw_readl(dev, DW_IC_CLR_STOP_DET);
+ regmap_read(dev->map, DW_IC_CLR_STOP_DET, &dummy);
if (stat & DW_IC_INTR_START_DET)
- dw_readl(dev, DW_IC_CLR_START_DET);
+ regmap_read(dev->map, DW_IC_CLR_START_DET, &dummy);
if (stat & DW_IC_INTR_GEN_CALL)
- dw_readl(dev, DW_IC_CLR_GEN_CALL);
+ regmap_read(dev->map, DW_IC_CLR_GEN_CALL, &dummy);
return stat;
}
@@ -155,14 +156,14 @@ static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
{
- u32 raw_stat, stat, enabled;
- u8 val, slave_activity;
+ u32 raw_stat, stat, enabled, tmp;
+ u8 val = 0, slave_activity;
- stat = dw_readl(dev, DW_IC_INTR_STAT);
- enabled = dw_readl(dev, DW_IC_ENABLE);
- raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
- DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
+ regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+ regmap_read(dev->map, DW_IC_ENABLE, &enabled);
+ regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
+ regmap_read(dev->map, DW_IC_STATUS, &tmp);
+ slave_activity = ((tmp & DW_IC_STATUS_SLAVE_ACTIVITY) >> 6);
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
@@ -177,7 +178,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
if (stat & DW_IC_INTR_RX_FULL) {
- val = dw_readl(dev, DW_IC_DATA_CMD);
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ val = tmp;
if (!i2c_slave_event(dev->slave,
I2C_SLAVE_WRITE_RECEIVED,
@@ -185,24 +187,24 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
dev_vdbg(dev->dev, "Byte %X acked!",
val);
}
- dw_readl(dev, DW_IC_CLR_RD_REQ);
+ regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
- dw_readl(dev, DW_IC_CLR_RD_REQ);
- dw_readl(dev, DW_IC_CLR_RX_UNDER);
+ regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
+ regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
I2C_SLAVE_READ_REQUESTED,
&val))
- dw_writel(dev, val, DW_IC_DATA_CMD);
+ regmap_write(dev->map, DW_IC_DATA_CMD, val);
}
}
if (stat & DW_IC_INTR_RX_DONE) {
if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
&val))
- dw_readl(dev, DW_IC_CLR_RX_DONE);
+ regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
stat = i2c_dw_read_clear_intrbits_slave(dev);
@@ -210,7 +212,8 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
}
if (stat & DW_IC_INTR_RX_FULL) {
- val = dw_readl(dev, DW_IC_DATA_CMD);
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ val = tmp;
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
&val))
dev_vdbg(dev->dev, "Byte %X acked!", val);
@@ -263,7 +266,7 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
dev->disable = i2c_dw_disable;
dev->disable_int = i2c_dw_disable_int;
- ret = i2c_dw_set_reg_access(dev);
+ ret = i2c_dw_init_regmap(dev);
if (ret)
return ret;
@@ -271,7 +274,9 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
if (ret)
return ret;
- i2c_dw_set_fifo_size(dev);
+ ret = i2c_dw_set_fifo_size(dev);
+ if (ret)
+ return ret;
ret = dev->init(dev);
if (ret)
--
2.26.2
^ permalink raw reply related
* [PATCH v6 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Baikal-T1 System Controller is equipped with a dedicated I2C Controller
which functionality is based on the DW APB I2C IP-core, the only
difference in a way it' registers are accessed. There are three access
register provided in the System Controller registers map, which indirectly
address the normal DW APB I2C registers space. So in order to have the
Baikal-T1 System I2C Controller supported by the common DW APB I2C driver
we created a dedicated Dw I2C controller model quirk, which retrieves the
syscon regmap from the parental dt node and creates a new regmap based on
it.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch, which has been created due to declining the
glue-layer approach.
Changelog v4:
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
Changelog v6:
- Add comma in the last explicitly initialized member of the bt1_i2c_cfg
struct regmap_config instance.
---
drivers/i2c/busses/Kconfig | 3 +-
drivers/i2c/busses/i2c-designware-core.h | 3 +
drivers/i2c/busses/i2c-designware-platdrv.c | 78 ++++++++++++++++++++-
3 files changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 259e2325712a..0cf7aea30138 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -541,8 +541,9 @@ config I2C_DESIGNWARE_SLAVE
config I2C_DESIGNWARE_PLATFORM
tristate "Synopsys DesignWare Platform"
- select I2C_DESIGNWARE_CORE
depends on (ACPI && COMMON_CLK) || !ACPI
+ select I2C_DESIGNWARE_CORE
+ select MFD_SYSCON if MIPS_BAIKAL_T1
help
If you say yes to this option, support will be included for the
Synopsys DesignWare I2C adapter.
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index f5bbe3d6bcf8..556673a1f61b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -183,6 +183,7 @@ struct reset_control;
* struct dw_i2c_dev - private i2c-designware data
* @dev: driver model device node
* @map: IO registers map
+ * @sysmap: System controller registers map
* @base: IO registers pointer
* @ext: Extended IO registers pointer
* @cmd_complete: tx completion indicator
@@ -235,6 +236,7 @@ struct reset_control;
struct dw_i2c_dev {
struct device *dev;
struct regmap *map;
+ struct regmap *sysmap;
void __iomem *base;
void __iomem *ext;
struct completion cmd_complete;
@@ -290,6 +292,7 @@ struct dw_i2c_dev {
#define ACCESS_NO_IRQ_SUSPEND 0x00000002
#define MODEL_MSCC_OCELOT 0x00000100
+#define MODEL_BAIKAL_BT1 0x00000200
#define MODEL_MASK 0x00000f00
int i2c_dw_init_regmap(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 9d467fa0e163..b55c730f28cf 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -18,6 +18,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_data/i2c-designware.h>
@@ -25,6 +26,7 @@
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -58,6 +60,63 @@ MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
#endif
#ifdef CONFIG_OF
+#define BT1_I2C_CTL 0x100
+#define BT1_I2C_CTL_ADDR_MASK GENMASK(7, 0)
+#define BT1_I2C_CTL_WR BIT(8)
+#define BT1_I2C_CTL_GO BIT(31)
+#define BT1_I2C_DI 0x104
+#define BT1_I2C_DO 0x108
+
+static int bt1_i2c_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct dw_i2c_dev *dev = context;
+ int ret;
+
+ /*
+ * Note these methods shouldn't ever fail because the system controller
+ * registers are memory mapped. We check the return value just in case.
+ */
+ ret = regmap_write(dev->sysmap, BT1_I2C_CTL,
+ BT1_I2C_CTL_GO | (reg & BT1_I2C_CTL_ADDR_MASK));
+ if (ret)
+ return ret;
+
+ return regmap_read(dev->sysmap, BT1_I2C_DO, val);
+}
+
+static int bt1_i2c_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct dw_i2c_dev *dev = context;
+ int ret;
+
+ ret = regmap_write(dev->sysmap, BT1_I2C_DI, val);
+ if (ret)
+ return ret;
+
+ return regmap_write(dev->sysmap, BT1_I2C_CTL,
+ BT1_I2C_CTL_GO | BT1_I2C_CTL_WR | (reg & BT1_I2C_CTL_ADDR_MASK));
+}
+
+static struct regmap_config bt1_i2c_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .fast_io = true,
+ .reg_read = bt1_i2c_read,
+ .reg_write = bt1_i2c_write,
+ .max_register = DW_IC_COMP_TYPE,
+};
+
+static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
+{
+ dev->sysmap = syscon_node_to_regmap(dev->dev->of_node->parent);
+ if (IS_ERR(dev->sysmap))
+ return PTR_ERR(dev->sysmap);
+
+ dev->map = devm_regmap_init(dev->dev, NULL, dev, &bt1_i2c_cfg);
+ return PTR_ERR_OR_ZERO(dev->map);
+}
+
#define MSCC_ICPU_CFG_TWI_DELAY 0x0
#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE BIT(0)
#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER 0x4
@@ -90,10 +149,16 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
static const struct of_device_id dw_i2c_of_match[] = {
{ .compatible = "snps,designware-i2c", },
{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+ { .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
{},
};
MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
#else
+static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
+{
+ return -ENODEV;
+}
+
static inline int dw_i2c_of_configure(struct platform_device *pdev)
{
return -ENODEV;
@@ -111,10 +176,19 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
{
struct platform_device *pdev = to_platform_device(dev->dev);
+ int ret;
- dev->base = devm_platform_ioremap_resource(pdev, 0);
+ switch (dev->flags & MODEL_MASK) {
+ case MODEL_BAIKAL_BT1:
+ ret = bt1_i2c_request_regs(dev);
+ break;
+ default:
+ dev->base = devm_platform_ioremap_resource(pdev, 0);
+ ret = PTR_ERR_OR_ZERO(dev->base);
+ break;
+ }
- return PTR_ERR_OR_ZERO(dev->base);
+ return ret;
}
static int dw_i2c_plat_probe(struct platform_device *pdev)
--
2.26.2
^ permalink raw reply related
* [PATCH v6 10/11] i2c: designware: Move reg-space remapping into a dedicated function
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
This is a preparation patch before adding a quirk with custom registers
map creation required for the Baikal-T1 System I2C support.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch, which has been created due to declining the
glue-layer approach.
Changelog v4:
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() method.
- Discard devm_platform_get_and_ioremap_resource() utilization.
---
drivers/i2c/busses/i2c-designware-platdrv.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 38657d821c72..9d467fa0e163 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -108,6 +108,15 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
pm_runtime_put_noidle(dev->dev);
}
+static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev->dev);
+
+ dev->base = devm_platform_ioremap_resource(pdev, 0);
+
+ return PTR_ERR_OR_ZERO(dev->base);
+}
+
static int dw_i2c_plat_probe(struct platform_device *pdev)
{
struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -125,15 +134,14 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
return -ENOMEM;
dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
-
- dev->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(dev->base))
- return PTR_ERR(dev->base);
-
dev->dev = &pdev->dev;
dev->irq = irq;
platform_set_drvdata(pdev, dev);
+ ret = dw_i2c_plat_request_regs(dev);
+ if (ret)
+ return ret;
+
dev->rst = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
if (IS_ERR(dev->rst))
return PTR_ERR(dev->rst);
--
2.26.2
^ permalink raw reply related
* [PATCH v6 09/11] i2c: designware: Retrieve quirk flags as early as possible
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Some platforms might need to activate the driver quirks at a very early
probe stage. For instance, Baikal-T1 System I2C doesn't need to map the
registers space as ones belong to the system controller. Instead it will
request the syscon regmap from the parental DT node. In order to be able
to do so let's retrieve the model flags right after the DW I2C private
data is created. While at it replace the or-assignment with just
assignment operator since or-ing is redundant at this stage.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- This is a new patch, which has been created due to declining the
glue-layer approach.
Changelog v5:
- Replace or-assignment with just assignment operator.
---
drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index ca057aa9eac4..38657d821c72 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -124,6 +124,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
+ dev->flags = (uintptr_t)device_get_match_data(&pdev->dev);
+
dev->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(dev->base))
return PTR_ERR(dev->base);
@@ -146,8 +148,6 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
i2c_dw_acpi_adjust_bus_speed(&pdev->dev);
- dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev);
-
if (pdev->dev.of_node)
dw_i2c_of_configure(pdev);
--
2.26.2
^ permalink raw reply related
* [PATCH v6 04/11] i2c: designware: Use `-y` to build multi-object modules
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Andy Shevchenko, Mika Westerberg, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Since commit 4f8272802739 ("Documentation: update kbuild loadable modules
goals & examples") `-objs` is fitted for building host programs, lets
change DW I2C core, platform and PCI driver kbuild directives to using
`-y`, which more straightforward for device drivers. By doing so we can
discard the ifeq construction in favor to the more natural and less bulky
`<module>-$(CONFIG_X) += x.o`
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
drivers/i2c/busses/Makefile | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index a33aa107a05d..c6813d7b2780 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -48,17 +48,15 @@ obj-$(CONFIG_I2C_CADENCE) += i2c-cadence.o
obj-$(CONFIG_I2C_CBUS_GPIO) += i2c-cbus-gpio.o
obj-$(CONFIG_I2C_CPM) += i2c-cpm.o
obj-$(CONFIG_I2C_DAVINCI) += i2c-davinci.o
-obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
-i2c-designware-core-objs := i2c-designware-common.o
-i2c-designware-core-objs += i2c-designware-master.o
-ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
-i2c-designware-core-objs += i2c-designware-slave.o
-endif
-obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o
-i2c-designware-platform-objs := i2c-designware-platdrv.o
+obj-$(CONFIG_I2C_DESIGNWARE_CORE) += i2c-designware-core.o
+i2c-designware-core-y := i2c-designware-common.o
+i2c-designware-core-y += i2c-designware-master.o
+i2c-designware-core-$(CONFIG_I2C_DESIGNWARE_SLAVE) += i2c-designware-slave.o
+obj-$(CONFIG_I2C_DESIGNWARE_PLATFORM) += i2c-designware-platform.o
+i2c-designware-platform-y := i2c-designware-platdrv.o
i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-baytrail.o
-obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
-i2c-designware-pci-objs := i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_DESIGNWARE_PCI) += i2c-designware-pci.o
+i2c-designware-pci-y := i2c-designware-pcidrv.o
obj-$(CONFIG_I2C_DIGICOLOR) += i2c-digicolor.o
obj-$(CONFIG_I2C_EFM32) += i2c-efm32.o
obj-$(CONFIG_I2C_EG20T) += i2c-eg20t.o
--
2.26.2
^ permalink raw reply related
* [PATCH v6 07/11] i2c: designware: Discard Cherry Trail model flag
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Andy Shevchenko, Mika Westerberg
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Rob Herring, linux-mips, devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
A PM workaround activated by the flag MODEL_CHERRYTRAIL has been removed
since commit 9cbeeca05049 ("i2c: designware: Remove Cherry Trail PMIC I2C
bus pm_disabled workaround"), but the flag most likely by mistake has been
left in the Dw I2C drivers. Let's remove it. Since MODEL_MSCC_OCELOT is
the only model-flag left, redefine it to be 0x100 so setting a very first
bit in the MODEL_MASK bits range.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- Since MSCC and Baikal-T1 will be a part of the platform driver code, we
have to preserve the MODEL_MASK macro to use it to filter the model
flags during the IP-specific quirks activation.
---
drivers/i2c/busses/i2c-designware-core.h | 3 +--
drivers/i2c/busses/i2c-designware-pcidrv.c | 1 -
drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 150de5e5c31b..b9ef9b0deef0 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -289,8 +289,7 @@ struct dw_i2c_dev {
#define ACCESS_INTR_MASK 0x00000004
#define ACCESS_NO_IRQ_SUSPEND 0x00000008
-#define MODEL_CHERRYTRAIL 0x00000100
-#define MODEL_MSCC_OCELOT 0x00000200
+#define MODEL_MSCC_OCELOT 0x00000100
#define MODEL_MASK 0x00000f00
u32 dw_readl(struct dw_i2c_dev *dev, int offset);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 11a5e4751eab..947c096f86e3 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -149,7 +149,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
},
[cherrytrail] = {
.bus_num = -1,
- .flags = MODEL_CHERRYTRAIL,
.scl_sda_cfg = &byt_config,
},
[elkhartlake] = {
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index f6d2c96e35ce..ca057aa9eac4 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -44,7 +44,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
{ "INT3432", 0 },
{ "INT3433", 0 },
{ "80860F41", ACCESS_NO_IRQ_SUSPEND },
- { "808622C1", ACCESS_NO_IRQ_SUSPEND | MODEL_CHERRYTRAIL },
+ { "808622C1", ACCESS_NO_IRQ_SUSPEND },
{ "AMD0010", ACCESS_INTR_MASK },
{ "AMDI0010", ACCESS_INTR_MASK },
{ "AMDI0510", 0 },
--
2.26.2
^ permalink raw reply related
* [PATCH v6 05/11] i2c: designware: slave: Set DW I2C core module dependency
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Andy Shevchenko, Mika Westerberg, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
DW APB I2C slave code in fact depends on the DW I2C driver core, but not
on the platform code as it used to be before commit 90bc1ee6de9f ("i2c:
designware: Allow slave mode for PCI enumerated devices"). Yes, the I2C
slave interface is currently supported by both the platform and PCI
versions of the IP core, but it still depends on the DW I2C core
functionality and must be available only if the last one is enabled.
So make sure the DW APB I2C slave config is only available if the
I2C_DESIGNWARE_CORE config is enabled.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
drivers/i2c/busses/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 1d940810e47e..7f92f6a96042 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -529,6 +529,7 @@ config I2C_DESIGNWARE_CORE
config I2C_DESIGNWARE_SLAVE
bool "Synopsys DesignWare Slave"
+ depends on I2C_DESIGNWARE_CORE
select I2C_SLAVE
help
If you say yes to this option, support will be included for the
--
2.26.2
^ permalink raw reply related
* [PATCH v6 00/11] i2c: designeware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko, Vadim Vlasov,
Alexey Kolotnikov, Thomas Bogendoerfer, Andy Shevchenko,
Mika Westerberg, Rob Herring, linux-mips, linux-i2c, devicetree,
linux-kernel
Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
the patchset.
Initially this has been a small patchset which embedded the Baikal-T1
System I2C support into the DW APB I2C driver as is by using a simplest
way. After a short discussion with Andy we decided to implement what he
suggested (introduce regmap-based accessors and create a glue driver) and
even more than that to provide some cleanups of the code. So here is what
this patchset consists of.
First of all we've found out that current implementation of scripts/dtc
didn't support i2c dt nodes with 10bit and slave flags set in the
reg property. You'll see an error if you try to dt_binding_check it.
So the very first patch fixes the problem by adding these flags support
into the check_i2c_bus_reg() method.
Traditionally we converted the plain text-based DT binding to the DT schema
and added Baikal-T1 System I2C device support there. This required to mark
the reg property redundant for Baikal-T1 I2C since its reg-space is
indirectly accessed by means of the System Controller cmd/read/write
registers.
Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
registers IO accessors into the regmap API methods. This doesn't change
the code logic much, though in two places we managed to replace some bulky
peaces of code with a ready-to-use regmap methods.
Additionally before adding the glue layer API we initiated a set of cleanups:
- Define components of the multi-object drivers (like i2c-designware-core.o
and i2c-designware-paltform.o) with using `-y` suffixed makefile
variables instead of `-objs` suffixed one. This is encouraged by
Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
to build host programs.
- Make DW I2C slave driver depended on the DW I2C core code instead of the
platform one, which it really is.
- Move Intel Baytrail semaphore feature to the platform if-clause of the
kernel config.
After this we finally can introduce the glue layer API for the DW APB I2C
platform driver. So there are three methods exported from the driver:
i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
setup, cleanup and add PM operations to the glue driven I2C device. Before
setting the platform DW I2C device up the glue probe code is supposed to
create an instance of DW I2C device generic object and pre-initialize
its `struct device` pointer together with optional platform-specific
flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
code into the glue layer seeing it's really too specific and, which is more
important, isn't that complicated so we could unpin it without much of
worrying to break something.
Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
were no longer used in the code. MODEL_MSCC flag has been discarded since
the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
rid of all the MODEL-specific flags.
Finally we introduced a glue driver with Baikal-T1 System I2C device
support. The driver probe tries to find a syscon regmap, creates the DW
APB I2C regmap based on it and passes it further to the DW I2C device
descriptor. Then it does normal DW APB I2C platform setup by calling a
generic setup method. Cleanup is straightforward. It's just calling a
generic DW APB I2C clean method.
This patchset is rebased and tested on the i2c/for-next (5.7-rc4):
base-commit: 2a41d0f91443 Merge branch 'i2c/for-5.8' into i2c/for-next
Note new vendor prefix for Baikal-T1 System I2C device is added in the
framework of the next patchset:
https://lkml.org/lkml/2020/5/6/1047
Changelog v2:
- Fix the SoB tags.
- Use a shorter summary describing the bindings convertion patch.
- Patch "i2c: designware: Detect the FIFO size in the common code" has
been acked by Jarkko and applied by Wolfram to for-next so drop it from
the set.
- Patch "i2c: designware: Discard i2c_dw_read_comp_param() function" has
been acked by Jarkko and applied by Wolfram to for-next so drop it from
the set.
- Make sure that "mscc,ocelot-i2c" compatible node may have up to two
registers space defined in the DT node, while normal DW I2C controller
will have only one registers space.
- Add "mscc,ocelot-i2c" DT schema example to test the previous fix.
- Declare "unevaluatedProperties" property instead of
"additionalProperties" one in the DT schema.
- Due to the previous fix we can now discard the dummy boolean properties
declaration, since the proper type evaluation will be performed by the
generic i2c-controller.yaml schema.
- Refactor the DW I2C APB driver related series to address the Andies
notes.
- Convert DW APB I2C driver to using regmap instead of handwritten
accessors.
- Use `-y` to build multi-object DW APB drivers.
- Fix DW APB I2C slave code dependency. It should depend on
I2C_DESIGNWARE_CORE instead I2C_DESIGNWARE_PLATFORM.
- Move Baytrail semaphore config to the platform if-clause.
- Introduce a glue-layer platform driver API.
- Unpin Microsemi Ocelot I2C code into a glue driver.
- Remove MODEL_CHERRYTRAIL and MODEL_MASK as no longer needed.
- Add support for custom regmap passed from glue driver.
- Add Baikal-T1 System I2C support in a dedicated glue layer driver.
Link: https://lore.kernel.org/linux-i2c/20200510095019.20981-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Move fixes and less invasive patches to the head of the series.
- Add patch "dt-bindings: i2c: Discard i2c-slave flag from the DW I2C
example" since Rob says the flag can be discarded until dtc is fixed.
- Add patch "i2c: designware: Retrieve quirk flags as early as possible"
as a first preparation before adding Baikal-T1 System I2C support.
- Add patch "i2c: designware: Move reg-space remapping into a dedicated
function" as a second preparation before adding Baikal-T1 System I2C
support.
- Add patch "i2c: designware: Add Baikal-T1 System I2C support", which
integrates the Baikal-T1 I2C support into the DW I2C platform driver.
- Get back the reg property being mandatory even if it's Baikal-T1 System
I2C DT node. Rob says it has to be in the DT node if there is a
dedicated registers range in the System Controller registers space.
- Replace if-endif clause around the I2C_DESIGNWARE_BAYTRAIL config
with "depends on" operator.
Link: https://lore.kernel.org/linux-i2c/20200526215528.16417-1-Sergey.Semin@baikalelectronics.ru/
Changelog v4:
- Rebase on top of the i2c/for-next branch.
- Use PTR_ERR_OR_ZERO() helper in the bt1_i2c_request_regs() and
in the dw_i2c_plat_request_regs() methods.
- Discard devm_platform_get_and_ioremap_resource() utilization.
- Discard patch "scripts/dtc: check: Add 10bit/slave i2c reg flags
support" since it must be merged in to the dtc upstream repository.
Link: https://lore.kernel.org/linux-i2c/20200527120111.5781-1-Sergey.Semin@baikalelectronics.ru
Changelog v5:
- Replace or-assignment with just assignment operator when getting
the quirk flags.
- Keep alphabetical order of the include statements.
- Discard explicit u16-type cast in the dw_reg_write_word() method.
Link: https://lore.kernel.org/linux-i2c/20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru
Changelog v6:
- Add commas after the last member of the regmap_config instances
initializers.
- Replace the "linux,slave-24c02" compatible string with "atmel,24c02" one
so the DT example would be perceived as a normal DW I2C master mode.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Serge Semin (11):
dt-bindings: i2c: Convert DW I2C binding to DT schema
dt-bindings: i2c: Convert DW I2C slave to the DW I2C master example
dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller
i2c: designware: Use `-y` to build multi-object modules
i2c: designware: slave: Set DW I2C core module dependency
i2c: designware: Add Baytrail sem config DW I2C platform dependency
i2c: designware: Discard Cherry Trail model flag
i2c: designware: Convert driver to using regmap API
i2c: designware: Retrieve quirk flags as early as possible
i2c: designware: Move reg-space remapping into a dedicated function
i2c: designware: Add Baikal-T1 System I2C support
.../bindings/i2c/i2c-designware.txt | 73 -------
.../bindings/i2c/snps,designware-i2c.yaml | 156 +++++++++++++++
drivers/i2c/busses/Kconfig | 28 +--
drivers/i2c/busses/Makefile | 18 +-
drivers/i2c/busses/i2c-designware-common.c | 178 +++++++++++++-----
drivers/i2c/busses/i2c-designware-core.h | 28 +--
drivers/i2c/busses/i2c-designware-master.c | 125 ++++++------
drivers/i2c/busses/i2c-designware-pcidrv.c | 1 -
drivers/i2c/busses/i2c-designware-platdrv.c | 96 +++++++++-
drivers/i2c/busses/i2c-designware-slave.c | 77 ++++----
10 files changed, 520 insertions(+), 260 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-designware.txt
create mode 100644 Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
--
2.26.2
^ permalink raw reply
* [PATCH v6 06/11] i2c: designware: Add Baytrail sem config DW I2C platform dependency
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang
Cc: Serge Semin, Serge Semin, Andy Shevchenko, Alexey Malahov,
Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C
platform driver. It's a bit confusing to see it's config in the menu at
some separated place with no reference to the platform code. Let's move the
config definition to be below the I2C_DESIGNWARE_PLATFORM config and mark
it with "depends on I2C_DESIGNWARE_PLATFORM" statement. By doing so the
config menu will display the feature right below the DW I2C platform
driver item and will indent it to the right so signifying its belonging.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Changelog v3:
- Replace if-endif clause around the I2C_DESIGNWARE_BAYTRAIL config
with "depends on" operator.
---
drivers/i2c/busses/Kconfig | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7f92f6a96042..7cd279c36898 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -549,20 +549,10 @@ config I2C_DESIGNWARE_PLATFORM
This driver can also be built as a module. If so, the module
will be called i2c-designware-platform.
-config I2C_DESIGNWARE_PCI
- tristate "Synopsys DesignWare PCI"
- depends on PCI
- select I2C_DESIGNWARE_CORE
- help
- If you say yes to this option, support will be included for the
- Synopsys DesignWare I2C adapter. Only master mode is supported.
-
- This driver can also be built as a module. If so, the module
- will be called i2c-designware-pci.
-
config I2C_DESIGNWARE_BAYTRAIL
bool "Intel Baytrail I2C semaphore support"
depends on ACPI
+ depends on I2C_DESIGNWARE_PLATFORM
depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
(I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
help
@@ -572,6 +562,17 @@ config I2C_DESIGNWARE_BAYTRAIL
the platform firmware controlling it. You should say Y if running on
a BayTrail system using the AXP288.
+config I2C_DESIGNWARE_PCI
+ tristate "Synopsys DesignWare PCI"
+ depends on PCI
+ select I2C_DESIGNWARE_CORE
+ help
+ If you say yes to this option, support will be included for the
+ Synopsys DesignWare I2C adapter. Only master mode is supported.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-designware-pci.
+
config I2C_DIGICOLOR
tristate "Conexant Digicolor I2C driver"
depends on ARCH_DIGICOLOR || COMPILE_TEST
--
2.26.2
^ permalink raw reply related
* [PATCH v6 02/11] dt-bindings: i2c: Convert DW I2C slave to the DW I2C master example
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Rob Herring
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Andy Shevchenko, Mika Westerberg, linux-mips, linux-i2c,
devicetree, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
dtc currently doesn't support I2C_OWN_SLAVE_ADDRESS flag set in the
i2c "reg" property. If dtc finds an i2c-slave sub-node having an address
higher than ten-bits wide it'll print an ugly warning:
Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64: I2C bus unit address format error, expected "40000064"
Warning (i2c_bus_reg): /example-2/i2c@1120000/eeprom@64:reg: I2C address must be less than 10-bits, got "0x40000064"
In order to silence dtc up let's replace the corresponding DT binding
example with a normal DW I2C master mode-based one. It's done by clearing
the I2C_OWN_SLAVE_ADDRESS bit in the reg property and converting the
sub-node to be compatible with normal EEPROM like "atmel,24c02".
Just revert this commit when dtc is fixed.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
Rob, even though you asked for such modification, it might be a better to
just ignore the warning until dtc is properly fixed. Andy and me agree
with that. If you are also on the same side with us, just explicitly nack
this patch so Jarkko or Wolfram would ignore it when merging in the series.
Changelog v3:
- This is a new patch created as a result of the Rob request to remove
the EEPROM-slave bit setting in the DT binndings example until the dtc
is fixed.
Changelog v6:
- Replace the "linux,slave-24c02" compatible string with "atmel,24c02" one
so the example would be perceived as a normal DW I2C master mode.
---
.../devicetree/bindings/i2c/snps,designware-i2c.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 4bd430b2b41d..dff3f267bdee 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -136,8 +136,8 @@ examples:
interrupts = <0>;
eeprom@64 {
- compatible = "linux,slave-24c02";
- reg = <0x40000064>;
+ compatible = "atmel,24c02";
+ reg = <0x64>;
};
};
- |
--
2.26.2
^ permalink raw reply related
* [PATCH v6 03/11] dt-bindings: i2c: dw: Add Baikal-T1 SoC I2C controller
From: Serge Semin @ 2020-05-28 9:33 UTC (permalink / raw)
To: Jarkko Nikula, Wolfram Sang, Rob Herring
Cc: Serge Semin, Serge Semin, Alexey Malahov, Thomas Bogendoerfer,
Andy Shevchenko, Mika Westerberg, linux-mips, linux-i2c,
devicetree, linux-kernel
In-Reply-To: <20200528093322.23553-1-Sergey.Semin@baikalelectronics.ru>
Add the "baikal,bt1-sys-i2c" compatible string to the DW I2C binding. Even
though the corresponding node is supposed to be a child of the Baikal-T1
System Controller, its reg property is left required for compatibility.
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: linux-mips@vger.kernel.org
---
Changelog v2:
- Make the reg property being optional if it's Baikal-T1 System I2C DT
node.
Changelog v3:
- Get back the reg property being mandatory even if it's Baikal-T1 System
I2C DT node. Rob says it has to be in the DT node if there is a
dedicated registers range in the System Controller registers space.
---
Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index dff3f267bdee..4f746bef2374 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -31,6 +31,8 @@ properties:
items:
- const: mscc,ocelot-i2c
- const: snps,designware-i2c
+ - description: Baikal-T1 SoC System I2C controller
+ const: baikal,bt1-sys-i2c
reg:
minItems: 1
--
2.26.2
^ permalink raw reply related
* Re: [PATCH 1/3] ARM: dts: r8a7742-iwg21d-q7: Enable HSUSB, USB2.0 and XHCI
From: Sergei Shtylyov @ 2020-05-28 9:36 UTC (permalink / raw)
To: Lad Prabhakar, Geert Uytterhoeven, Magnus Damm, Rob Herring
Cc: linux-renesas-soc, devicetree, linux-kernel, Prabhakar
In-Reply-To: <1590611013-26029-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
Hello!
On 27.05.2020 23:23, Lad Prabhakar wrote:
> Enable support for HSUB, USB2.0 and xhci on iWave RZ/G1H carrier board.
HSUSB, xHCI.
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
[...]
MBR, Sergei
^ 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