* Re: [PATCHv6 00/14] atmel-isi/ov7670/ov2640: convert to standalone drivers
From: Nicolas Ferre @ 2017-04-04 14:00 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Guennadi Liakhovetski, Songjun Wu, Sakari Ailus, devicetree,
Wu, Songjun
In-Reply-To: <20170328082347.11159-1-hverkuil@xs4all.nl>
Le 28/03/2017 à 10:23, Hans Verkuil a écrit :
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This patch series converts the soc-camera atmel-isi to a standalone V4L2
> driver.
>
> The same is done for the ov7670 and ov2640 sensor drivers: the ov7670 was
> used to test the atmel-isi driver. The ov2640 is needed because the em28xx
> driver has a soc_camera include dependency. Both ov7670 and ov2640 sensors
> have been tested with the atmel-isi driver.
>
> The first 5 patches improve the ov7670 sensor driver, mostly adding modern
> features such as DT support.
>
> The next three convert the atmel-isi and move it out of soc_camera.
>
> The following 6 patches convert ov2640 and drop the soc_camera dependency
> in em28xx. I have tested that this works with my 'SpeedLink Vicious And
> Divine Laplace webcam'.
>
> Tested with my sama5d3-Xplained board, the ov2640 sensor and two ov7670
> sensors: one with and one without reset/pwdn pins. Also tested with my
> em28xx-based webcam.
>
> I'd like to get this in for 4.12. Fingers crossed.
>
> Regards,
>
> Hans
>
> Changes since v5:
> - Dropped the last two dts patches as these were for demonstration purposes
> only.
> - Simplified isi_graph_init() return handling as suggested by Sakari.
> - Simplified atmel-isi format handling as suggested by Guennadi. Thanks for
> the suggestion, this improves the code nicely!
> - Improved RGB handling in atmel-isi, allowing for all YUV ordering and not
> just YUYV. Tested with YUYV and UYVY (the only two I can test with my
> hardware).
> - Improved commit message of the "atmel-isi: document device tree bindings"
> patch and dropped unnecessary properties from the example as per Rob's
> comments.
>
> Changes since v4:
> - the ov2640 colorspace fixes were inexplicably part of an atmel-isi patch.
> Split it off as a separate patch.
> - add V4L2_SUBDEV_FL_HAS_DEVNODE to ov2640.
> - drop #if defined(CONFIG_MEDIA_CONTROLLER) guard around media_entity_cleanup
> in ov2640.
>
> Changes since v3:
> - ov2640/ov7670: call clk_disable_unprepare where needed. I assumed this was
> done by the devm_clk_get cleanup, but that wasn't the case.
> - bindings: be even more explicit about which properties are mandatory.
> - ov2640/ov7670: drop unused bus-width from the dts binding examples and from
> the actual dts patches.
>
> Changes since v2:
> - Incorporated Sakari's and Rob's device tree bindings comments.
> - ov2640: dropped the reset/power changes. These actually broke the em28xx
> and there was really nothing wrong with it.
> - merged the "ov2640: allow use inside em28xx" into patches 10 and 11.
> It really shouldn't have been a separate patch in the first place.
> - rebased on top of 4.11-rc1.
>
> Changes since v1:
>
> - Dropped MC support from atmel-isi and ov7670: not needed to make this
> work. Only for the ov2640 was it kept since the em28xx driver requires it.
> - Use devm_clk_get instead of clk_get.
> - The ov7670 lower limit of the clock speed is 10 MHz instead of 12. Adjust
> accordingly.
>
>
> Hans Verkuil (14):
> ov7670: document device tree bindings
> ov7670: call v4l2_async_register_subdev
> ov7670: fix g/s_parm
> ov7670: get xclk
> ov7670: add devicetree support
> atmel-isi: update device tree bindings documentation
> atmel-isi: remove dependency of the soc-camera framework
> atmel-isi: move out of soc_camera to atmel
> ov2640: fix colorspace handling
> ov2640: update bindings
> ov2640: convert from soc-camera to a standard subdev sensor driver.
> ov2640: use standard clk and enable it.
> ov2640: add MC support
> em28xx: drop last soc_camera link
For the record, here is my test tag:
Tested-by: Nicolas Ferre <nicolas.ferre@microchip.com>
On a Microchip/Atmel SAMA5D4-EK with OV2640 and the per-existing OV7740.
Thanks a lot Hans for this valuable work!
Best regards,
> .../devicetree/bindings/media/atmel-isi.txt | 91 +-
> .../devicetree/bindings/media/i2c/ov2640.txt | 23 +-
> .../devicetree/bindings/media/i2c/ov7670.txt | 43 +
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 11 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/{soc_camera => }/ov2640.c | 153 +--
> drivers/media/i2c/ov7670.c | 75 +-
> drivers/media/i2c/soc_camera/Kconfig | 6 -
> drivers/media/i2c/soc_camera/Makefile | 1 -
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/atmel/Kconfig | 11 +-
> drivers/media/platform/atmel/Makefile | 1 +
> drivers/media/platform/atmel/atmel-isi.c | 1368 ++++++++++++++++++++
> .../platform/{soc_camera => atmel}/atmel-isi.h | 0
> drivers/media/platform/soc_camera/Kconfig | 11 -
> drivers/media/platform/soc_camera/Makefile | 1 -
> drivers/media/platform/soc_camera/atmel-isi.c | 1167 -----------------
> drivers/media/usb/em28xx/em28xx-camera.c | 9 -
> 19 files changed, 1614 insertions(+), 1360 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7670.txt
> rename drivers/media/i2c/{soc_camera => }/ov2640.c (92%)
> create mode 100644 drivers/media/platform/atmel/atmel-isi.c
> rename drivers/media/platform/{soc_camera => atmel}/atmel-isi.h (100%)
> delete mode 100644 drivers/media/platform/soc_camera/atmel-isi.c
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH V3 1/6] ARM: tegra: Remove unnecessary inclusion of flowctrl header
From: Thierry Reding @ 2017-04-04 13:41 UTC (permalink / raw)
To: Jon Hunter
Cc: Rob Herring, Mark Rutland, Russell King, Stephen Warren,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490704978-22906-2-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 407 bytes --]
On Tue, Mar 28, 2017 at 01:42:53PM +0100, Jon Hunter wrote:
> The Tegra flowctrl.h header is included unnecessarily by the Tegra
> sleep.S source file. Remove this unnecessary inclusion.
>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm/mach-tegra/sleep.S | 2 --
> 1 file changed, 2 deletions(-)
Applied to for-4.12/arm/soc, thanks.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] clk: vc5: Add support for IDT VersaClock 5P49V5935
From: Alexey Firago @ 2017-04-04 13:36 UTC (permalink / raw)
To: Geert Uytterhoeven, Marek Vasut
Cc: Michael Turquette, Stephen Boyd, Rob Herring, linux-clk,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMuHMdUOtY5i0_BoS7cKYzWKpA_fF3VssN4bZnNyFB117EpacQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04.04.2017 16:23, Geert Uytterhoeven wrote:
> On Tue, Apr 4, 2017 at 3:21 PM, Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 04/04/2017 03:16 PM, Alexey Firago wrote:
>>> + switch (vc5->model) {
>>> + case IDT_VC5_5P49V5923:
>>> + case IDT_VC5_5P49V5933:
>>> + vc5->clk_fod_cnt = 2;
>>> + vc5->clk_out_cnt = 3;
>>> + break;
>>> + case IDT_VC5_5P49V5935:
>>> + vc5->clk_fod_cnt = 4;
>>> + vc5->clk_out_cnt = 5;
>>> + break;
>>> + default:
>>> + /* Should never go here */
>>> + dev_err(&client->dev, "unsupported IDT VC5 ID specified\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> vc5->pin_xin = devm_clk_get(&client->dev, "xin");
>>> if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
>>> return -EPROBE_DEFER;
>
>>> @@ -744,7 +772,7 @@ static int vc5_probe(struct i2c_client *client,
>>> return 0;
>>>
>>> err_clk:
>>> - if (vc5->model == IDT_VC5_5P49V5933)
>>> + if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
>>
>> Maybe we should introduce some sort of flags to describe the VC
>> properties instead of using the model all over the place ?
>
> Yep, a structure describing the features (incl. fod_cnt and out_cnt), to be
> pointed to by clk_vc5_of_match[].data.
Ok, will rework and resend.
Thanks,
Alexey
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] clk: vc5: Add support for IDT VersaClock 5P49V5935
From: Alexey Firago @ 2017-04-04 13:35 UTC (permalink / raw)
To: Marek Vasut, mturquette-rdvid1DuHRBWk0Htik3J/w,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <72c66927-0acf-c888-2b4b-906a6c27e15c-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 04.04.2017 16:21, Marek Vasut wrote:
> On 04/04/2017 03:16 PM, Alexey Firago wrote:
>> Update IDT VersaClock 5 driver to support 5P49V5935. This chip has
>> two clock inputs (internal XTAL or external CLKIN), four fractional
>> dividers (FODs) and five clock outputs (four universal clock outputs
>> and one reference clock output at OUT0_SELB_I2C).
>>
>> Current driver supports up to 2 FODs and up to 3 clock outputs.
>> This patch sets max number of supported FODs to 4 and max number
>> of supported clocks to 5.
>>
>> Number of used FODs and clock outputs is set on probe according to
>> the model specified via device-tree.
>>
>> Signed-off-by: Alexey Firago <alexey_firago-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/clk/clk-versaclock5.c | 50 ++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 40 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
>> index 56741f3..f74d6e4 100644
>> --- a/drivers/clk/clk-versaclock5.c
>> +++ b/drivers/clk/clk-versaclock5.c
>> @@ -113,10 +113,17 @@
>> #define VC5_MUX_IN_XIN BIT(0)
>> #define VC5_MUX_IN_CLKIN BIT(1)
>>
>> +/* Maximum number of clk_out supported by this driver */
>> +#define VC5_MAX_CLK_OUT_NUM 5
>> +
>> +/* Maximum number of FODs supported by this driver */
>> +#define VC5_MAX_FOD_NUM 4
>> +
>> /* Supported IDT VC5 models. */
>> enum vc5_model {
>> IDT_VC5_5P49V5923,
>> IDT_VC5_5P49V5933,
>> + IDT_VC5_5P49V5935,
>> };
>>
>> struct vc5_driver_data;
>> @@ -139,8 +146,10 @@ struct vc5_driver_data {
>> unsigned char clk_mux_ins;
>> struct clk_hw clk_mux;
>> struct vc5_hw_data clk_pll;
>> - struct vc5_hw_data clk_fod[2];
>> - struct vc5_hw_data clk_out[3];
>> + int clk_fod_cnt;
>> + struct vc5_hw_data clk_fod[VC5_MAX_FOD_NUM];
>> + int clk_out_cnt;
>> + struct vc5_hw_data clk_out[VC5_MAX_CLK_OUT_NUM];
>> };
>>
>> static const char * const vc5_mux_names[] = {
>> @@ -563,7 +572,7 @@ static struct clk_hw *vc5_of_clk_get(struct of_phandle_args *clkspec,
>> struct vc5_driver_data *vc5 = data;
>> unsigned int idx = clkspec->args[0];
>>
>> - if (idx > 2)
>> + if (idx > vc5->clk_out_cnt)
>> return ERR_PTR(-EINVAL);
>>
>> return &vc5->clk_out[idx].hw;
>> @@ -576,6 +585,7 @@ static int vc5_map_index_to_output(const enum vc5_model model,
>> case IDT_VC5_5P49V5933:
>> return (n == 0) ? 0 : 3;
>> case IDT_VC5_5P49V5923:
>> + case IDT_VC5_5P49V5935:
>> default:
>> return n;
>> }
>> @@ -591,7 +601,7 @@ static int vc5_probe(struct i2c_client *client,
>> struct vc5_driver_data *vc5;
>> struct clk_init_data init;
>> const char *parent_names[2];
>> - unsigned int n, idx;
>> + unsigned int n, idx = 0;
>> int ret;
>>
>> vc5 = devm_kzalloc(&client->dev, sizeof(*vc5), GFP_KERNEL);
>> @@ -602,6 +612,23 @@ static int vc5_probe(struct i2c_client *client,
>> vc5->client = client;
>> vc5->model = (enum vc5_model)of_id->data;
>>
>> + /* Set number of supported outputs according to the repoted model */
>
> s/repoted/reported/ :)
Oops. Thanks, will fix.
>
>> + switch (vc5->model) {
>> + case IDT_VC5_5P49V5923:
>> + case IDT_VC5_5P49V5933:
>> + vc5->clk_fod_cnt = 2;
>> + vc5->clk_out_cnt = 3;
>> + break;
>> + case IDT_VC5_5P49V5935:
>> + vc5->clk_fod_cnt = 4;
>> + vc5->clk_out_cnt = 5;
>> + break;
>> + default:
>> + /* Should never go here */
>> + dev_err(&client->dev, "unsupported IDT VC5 ID specified\n");
>> + return -EINVAL;
>> + }
>> +
>> vc5->pin_xin = devm_clk_get(&client->dev, "xin");
>> if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
>> return -EPROBE_DEFER;
>> @@ -622,8 +649,9 @@ static int vc5_probe(struct i2c_client *client,
>> if (!IS_ERR(vc5->pin_xin)) {
>> vc5->clk_mux_ins |= VC5_MUX_IN_XIN;
>> parent_names[init.num_parents++] = __clk_get_name(vc5->pin_xin);
>> - } else if (vc5->model == IDT_VC5_5P49V5933) {
>> - /* IDT VC5 5P49V5933 has built-in oscilator. */
>> + } else if (vc5->model == IDT_VC5_5P49V5933 ||
>> + vc5->model == IDT_VC5_5P49V5935) {
>> + /* IDT VC5 5P49V5933 and 5P49V5935 have built-in oscilator. */
>> vc5->pin_xin = clk_register_fixed_rate(&client->dev,
>> "internal-xtal", NULL,
>> 0, 25000000);
>> @@ -672,7 +700,7 @@ static int vc5_probe(struct i2c_client *client,
>> }
>>
>> /* Register FODs */
>> - for (n = 0; n < 2; n++) {
>> + for (n = 0; n < vc5->clk_fod_cnt; n++) {
>> idx = vc5_map_index_to_output(vc5->model, n);
>> memset(&init, 0, sizeof(init));
>> init.name = vc5_fod_names[idx];
>> @@ -709,7 +737,7 @@ static int vc5_probe(struct i2c_client *client,
>> }
>>
>> /* Register FOD-connected OUTx outputs */
>> - for (n = 1; n < 3; n++) {
>> + for (n = 1; n < vc5->clk_out_cnt; n++) {
>> idx = vc5_map_index_to_output(vc5->model, n - 1);
>> parent_names[0] = vc5_fod_names[idx];
>> if (n == 1)
>> @@ -744,7 +772,7 @@ static int vc5_probe(struct i2c_client *client,
>> return 0;
>>
>> err_clk:
>> - if (vc5->model == IDT_VC5_5P49V5933)
>> + if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
>
> Maybe we should introduce some sort of flags to describe the VC
> properties instead of using the model all over the place ?
That makes sense. Will rework.
Regards,
Alexey
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V4 2/2] regulator: Add settling time for non-linear voltage transition
From: Laxman Dewangan @ 2017-04-04 13:29 UTC (permalink / raw)
To: broonie, lgirdwood, robh+dt
Cc: mark.rutland, linux-kernel, devicetree, Laxman Dewangan
In-Reply-To: <1491312590-20887-1-git-send-email-ldewangan@nvidia.com>
Some regulators (some PWM regulators) have the voltage transition
non-linear i.e. exponentially. On such cases, the settling time
for voltage transition can not be presented in the voltage-ramp-delay.
Add new property for non-linear voltage transition and handle this
in getting the voltage settling time.
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch is continuation of discussion on patch
regulator: pwm: Fix regulator ramp delay for continuous mode
https://patchwork.kernel.org/patch/9216857/
where is it discussed to have separate property for PWM which has
exponential voltage transition.
Changes from V1:
- Use new DT property to finding that voltage ramp is exponential or
not and use flag for having fixed delay for all voltage change.
Changes from V2:
- Based on review comment from V1, make the settling time property
independent of the regulator-ramp-delay and move this to core
framework instead of handling in PWM regulator.
Changes from V3:
- The change was sent long back and resuming this patch.
- Rebase on linux-next tag 20170330 for resend.
---
drivers/regulator/core.c | 2 ++
drivers/regulator/of_regulator.c | 4 ++++
include/linux/regulator/machine.h | 3 +++
3 files changed, 9 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 0753635..7303454 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2773,6 +2773,8 @@ static int _regulator_set_voltage_time(struct regulator_dev *rdev,
ramp_delay = rdev->constraints->ramp_delay;
else if (rdev->desc->ramp_delay)
ramp_delay = rdev->desc->ramp_delay;
+ else if (rdev->constraints->settling_time)
+ return rdev->constraints->settling_time;
if (ramp_delay == 0) {
rdev_dbg(rdev, "ramp_delay not set\n");
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..09d677d 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -86,6 +86,10 @@ static void of_get_regulation_constraints(struct device_node *np,
constraints->ramp_disable = true;
}
+ ret = of_property_read_u32(np, "regulator-settling-time-us", &pval);
+ if (!ret)
+ constraints->settling_time = pval;
+
ret = of_property_read_u32(np, "regulator-enable-ramp-delay", &pval);
if (!ret)
constraints->enable_time = pval;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index c9f795e..117699d 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -108,6 +108,8 @@ struct regulator_state {
* @initial_state: Suspend state to set by default.
* @initial_mode: Mode to set at startup.
* @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @settling_time: Time to settle down after voltage change when voltage
+ * change is non-linear (unit: microseconds).
* @active_discharge: Enable/disable active discharge. The enum
* regulator_active_discharge values are used for
* initialisation.
@@ -149,6 +151,7 @@ struct regulation_constraints {
unsigned int initial_mode;
unsigned int ramp_delay;
+ unsigned int settling_time;
unsigned int enable_time;
unsigned int active_discharge;
--
2.1.4
^ permalink raw reply related
* [PATCH V4 1/2] regulator: DT: Add settling time property for non-linear voltage change
From: Laxman Dewangan @ 2017-04-04 13:29 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Laxman Dewangan
Some regulators (some PWM regulators) have the voltage transition
exponentially. On such cases, the settling time for voltage change
is treated as constant time.
Add DT property for providing the settling time for any level of
voltage change for non-linear voltage change.
signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
This patch is continuation of discussion on patch
regulator: pwm: Fix regulator ramp delay for continuous mode
https://patchwork.kernel.org/patch/9216857/
where is it discussed to have separate property for PWM which has
exponential voltage transition.
Changes from V1:
- Pass the flag to tell that voltage ramp is exponential instead of
providing delay.
Changes from V2:
- Based on review comment from V1, make the settling time property
independent of the regulator-ramp-delay and move this out of PWM
regulator.
Changes from V3:
- The change was sent long back and resuming this patch.
- Added Rob's ack from V3 and content is same as V3.
https://lkml.org/lkml/2016/11/30/632
- Rebase on linux-next tag 20170330 for resend.
---
Documentation/devicetree/bindings/regulator/regulator.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 6ab5aef..d18edb0 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,9 @@ Optional properties:
design requires. This property describes the total system ramp time
required due to the combination of internal ramping of the regulator itself,
and board design issues such as trace capacitance and load on the supply.
+- regulator-settling-time-us: Settling time, in microseconds, for voltage
+ change if regulator have the constant time for any level voltage change.
+ This is useful when regulator have exponential voltage change.
- regulator-soft-start: Enable soft start so that voltage ramps slowly
- regulator-state-mem sub-root node for Suspend-to-RAM mode
: suspend to memory, the device goes to sleep, but all data stored in memory,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 2/2] clk: vc5: Add support for IDT VersaClock 5P49V5935
From: Geert Uytterhoeven @ 2017-04-04 13:23 UTC (permalink / raw)
To: Marek Vasut
Cc: Alexey Firago, Michael Turquette, Stephen Boyd, Rob Herring,
linux-clk, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <72c66927-0acf-c888-2b4b-906a6c27e15c-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, Apr 4, 2017 at 3:21 PM, Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>On 04/04/2017 03:16 PM, Alexey Firago wrote:
>> + switch (vc5->model) {
>> + case IDT_VC5_5P49V5923:
>> + case IDT_VC5_5P49V5933:
>> + vc5->clk_fod_cnt = 2;
>> + vc5->clk_out_cnt = 3;
>> + break;
>> + case IDT_VC5_5P49V5935:
>> + vc5->clk_fod_cnt = 4;
>> + vc5->clk_out_cnt = 5;
>> + break;
>> + default:
>> + /* Should never go here */
>> + dev_err(&client->dev, "unsupported IDT VC5 ID specified\n");
>> + return -EINVAL;
>> + }
>> +
>> vc5->pin_xin = devm_clk_get(&client->dev, "xin");
>> if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
>> return -EPROBE_DEFER;
>> @@ -744,7 +772,7 @@ static int vc5_probe(struct i2c_client *client,
>> return 0;
>>
>> err_clk:
>> - if (vc5->model == IDT_VC5_5P49V5933)
>> + if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
>
> Maybe we should introduce some sort of flags to describe the VC
> properties instead of using the model all over the place ?
Yep, a structure describing the features (incl. fod_cnt and out_cnt), to be
pointed to by clk_vc5_of_match[].data.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] arm64: tegra: Add GPU node for Tegra 186
From: Thierry Reding @ 2017-04-04 13:22 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Jonathan Hunter, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20170330092828.13336-1-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 375 bytes --]
On Thu, Mar 30, 2017 at 06:28:28PM +0900, Alexandre Courbot wrote:
> Add the DT node for the GP10B GPU on Tegra 186.
>
> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm64/boot/dts/nvidia/tegra186.dtsi | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
Applied to for-4.12/arm64/dt, thanks.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: Add documentation for GP10B GPU
From: Thierry Reding @ 2017-04-04 13:21 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Jonathan Hunter, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, gnurou-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20170330092644.11890-1-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 588 bytes --]
On Thu, Mar 30, 2017 at 06:26:44PM +0900, Alexandre Courbot wrote:
> GP10B's definition is mostly similar to GK20A's and GM20B's. The only
> noticeable difference is the use of power domains instead of a regulator
> for power supply.
>
> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> Changes since v1:
> - It's much better when it compiles.
>
> .../devicetree/bindings/gpu/nvidia,gk20a.txt | 25 +++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
Applied to for-4.12/arm64/dt, thanks.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] clk: vc5: Add support for IDT VersaClock 5P49V5935
From: Marek Vasut @ 2017-04-04 13:21 UTC (permalink / raw)
To: Alexey Firago, mturquette, sboyd, robh+dt, linux-clk, devicetree
In-Reply-To: <1491311788-31905-3-git-send-email-alexey_firago@mentor.com>
On 04/04/2017 03:16 PM, Alexey Firago wrote:
> Update IDT VersaClock 5 driver to support 5P49V5935. This chip has
> two clock inputs (internal XTAL or external CLKIN), four fractional
> dividers (FODs) and five clock outputs (four universal clock outputs
> and one reference clock output at OUT0_SELB_I2C).
>
> Current driver supports up to 2 FODs and up to 3 clock outputs.
> This patch sets max number of supported FODs to 4 and max number
> of supported clocks to 5.
>
> Number of used FODs and clock outputs is set on probe according to
> the model specified via device-tree.
>
> Signed-off-by: Alexey Firago <alexey_firago@mentor.com>
> ---
> drivers/clk/clk-versaclock5.c | 50 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
> index 56741f3..f74d6e4 100644
> --- a/drivers/clk/clk-versaclock5.c
> +++ b/drivers/clk/clk-versaclock5.c
> @@ -113,10 +113,17 @@
> #define VC5_MUX_IN_XIN BIT(0)
> #define VC5_MUX_IN_CLKIN BIT(1)
>
> +/* Maximum number of clk_out supported by this driver */
> +#define VC5_MAX_CLK_OUT_NUM 5
> +
> +/* Maximum number of FODs supported by this driver */
> +#define VC5_MAX_FOD_NUM 4
> +
> /* Supported IDT VC5 models. */
> enum vc5_model {
> IDT_VC5_5P49V5923,
> IDT_VC5_5P49V5933,
> + IDT_VC5_5P49V5935,
> };
>
> struct vc5_driver_data;
> @@ -139,8 +146,10 @@ struct vc5_driver_data {
> unsigned char clk_mux_ins;
> struct clk_hw clk_mux;
> struct vc5_hw_data clk_pll;
> - struct vc5_hw_data clk_fod[2];
> - struct vc5_hw_data clk_out[3];
> + int clk_fod_cnt;
> + struct vc5_hw_data clk_fod[VC5_MAX_FOD_NUM];
> + int clk_out_cnt;
> + struct vc5_hw_data clk_out[VC5_MAX_CLK_OUT_NUM];
> };
>
> static const char * const vc5_mux_names[] = {
> @@ -563,7 +572,7 @@ static struct clk_hw *vc5_of_clk_get(struct of_phandle_args *clkspec,
> struct vc5_driver_data *vc5 = data;
> unsigned int idx = clkspec->args[0];
>
> - if (idx > 2)
> + if (idx > vc5->clk_out_cnt)
> return ERR_PTR(-EINVAL);
>
> return &vc5->clk_out[idx].hw;
> @@ -576,6 +585,7 @@ static int vc5_map_index_to_output(const enum vc5_model model,
> case IDT_VC5_5P49V5933:
> return (n == 0) ? 0 : 3;
> case IDT_VC5_5P49V5923:
> + case IDT_VC5_5P49V5935:
> default:
> return n;
> }
> @@ -591,7 +601,7 @@ static int vc5_probe(struct i2c_client *client,
> struct vc5_driver_data *vc5;
> struct clk_init_data init;
> const char *parent_names[2];
> - unsigned int n, idx;
> + unsigned int n, idx = 0;
> int ret;
>
> vc5 = devm_kzalloc(&client->dev, sizeof(*vc5), GFP_KERNEL);
> @@ -602,6 +612,23 @@ static int vc5_probe(struct i2c_client *client,
> vc5->client = client;
> vc5->model = (enum vc5_model)of_id->data;
>
> + /* Set number of supported outputs according to the repoted model */
s/repoted/reported/ :)
> + switch (vc5->model) {
> + case IDT_VC5_5P49V5923:
> + case IDT_VC5_5P49V5933:
> + vc5->clk_fod_cnt = 2;
> + vc5->clk_out_cnt = 3;
> + break;
> + case IDT_VC5_5P49V5935:
> + vc5->clk_fod_cnt = 4;
> + vc5->clk_out_cnt = 5;
> + break;
> + default:
> + /* Should never go here */
> + dev_err(&client->dev, "unsupported IDT VC5 ID specified\n");
> + return -EINVAL;
> + }
> +
> vc5->pin_xin = devm_clk_get(&client->dev, "xin");
> if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> @@ -622,8 +649,9 @@ static int vc5_probe(struct i2c_client *client,
> if (!IS_ERR(vc5->pin_xin)) {
> vc5->clk_mux_ins |= VC5_MUX_IN_XIN;
> parent_names[init.num_parents++] = __clk_get_name(vc5->pin_xin);
> - } else if (vc5->model == IDT_VC5_5P49V5933) {
> - /* IDT VC5 5P49V5933 has built-in oscilator. */
> + } else if (vc5->model == IDT_VC5_5P49V5933 ||
> + vc5->model == IDT_VC5_5P49V5935) {
> + /* IDT VC5 5P49V5933 and 5P49V5935 have built-in oscilator. */
> vc5->pin_xin = clk_register_fixed_rate(&client->dev,
> "internal-xtal", NULL,
> 0, 25000000);
> @@ -672,7 +700,7 @@ static int vc5_probe(struct i2c_client *client,
> }
>
> /* Register FODs */
> - for (n = 0; n < 2; n++) {
> + for (n = 0; n < vc5->clk_fod_cnt; n++) {
> idx = vc5_map_index_to_output(vc5->model, n);
> memset(&init, 0, sizeof(init));
> init.name = vc5_fod_names[idx];
> @@ -709,7 +737,7 @@ static int vc5_probe(struct i2c_client *client,
> }
>
> /* Register FOD-connected OUTx outputs */
> - for (n = 1; n < 3; n++) {
> + for (n = 1; n < vc5->clk_out_cnt; n++) {
> idx = vc5_map_index_to_output(vc5->model, n - 1);
> parent_names[0] = vc5_fod_names[idx];
> if (n == 1)
> @@ -744,7 +772,7 @@ static int vc5_probe(struct i2c_client *client,
> return 0;
>
> err_clk:
> - if (vc5->model == IDT_VC5_5P49V5933)
> + if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
Maybe we should introduce some sort of flags to describe the VC
properties instead of using the model all over the place ?
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH v2 2/2] phy: meson: add USB2 PHY support for Meson GXL and GXM
From: Kishon Vijay Abraham I @ 2017-04-04 13:20 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: mark.rutland, devicetree, khilman, robh+dt, hendrik, carlo,
linux-amlogic, linux-arm-kernel
In-Reply-To: <CAFBinCAfO9_JfbCgHT3GmsKqoYhW-KqiT2s1f3WXRMO2LN4u_g@mail.gmail.com>
Hi,
On Saturday 01 April 2017 03:07 PM, Martin Blumenstingl wrote:
> Hi Kishon,
>
> On Wed, Mar 29, 2017 at 12:56 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi,
>>
>> On Wednesday 29 March 2017 03:03 AM, Martin Blumenstingl wrote:
>>> Hi Kishon,
>>>
>>> On Sat, Mar 18, 2017 at 2:00 PM, Martin Blumenstingl
>>> <martin.blumenstingl@googlemail.com> wrote:
>>>> This adds a new driver for the USB2 PHYs found on Meson GXL and GXM SoCs
>>>> (both SoCs are using the same USB PHY register layout).
>>>>
>>>> The USB2 PHY is a simple PHY which only has a few registers to configure
>>>> the mode (host/device) and a reset register (to enable/disable the PHY).
>>>>
>>>> Unfortunately there are no datasheets available for this PHY. The driver
>>>> was written by reading the code from Amlogic's GPL kernel sources and
>>>> by analyzing the registers on an actual GXL and GXM device running the
>>>> kernel that was shipped on the boards I have.
>>> gentle ping - did you have time to review this patch in v2 yet?
>>
>> Have a few comments, see below..
> thanks for taking the time to review this!
>
>>>
>>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>>> ---
>>>> drivers/phy/Kconfig | 14 ++
>>>> drivers/phy/Makefile | 1 +
>>>> drivers/phy/phy-meson-gxl-usb2.c | 273 +++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 288 insertions(+)
>>>> create mode 100644 drivers/phy/phy-meson-gxl-usb2.c
>>>>
>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>> index dc5277ad1b5a..2573e139fd17 100644
>>>> --- a/drivers/phy/Kconfig
>>>> +++ b/drivers/phy/Kconfig
>>>> @@ -510,6 +510,19 @@ config PHY_MESON8B_USB2
>>>> and GXBB SoCs.
>>>> If unsure, say N.
>>>>
>>>> +config PHY_MESON_GXL_USB
>>>> + tristate "Meson GXL and GXM USB2 PHY drivers"
>>>> + default ARCH_MESON
>>>> + depends on OF && (ARCH_MESON || COMPILE_TEST)
>>>> + depends on USB_SUPPORT
>>>> + select USB_COMMON
>>>> + select GENERIC_PHY
>>>> + select REGMAP_MMIO
>>>> + help
>>>> + Enable this to support the Meson USB2 PHYs found in Meson
>>>> + GXL and GXM SoCs.
>>>> + If unsure, say N.
>>>> +
>>>> config PHY_NSP_USB3
>>>> tristate "Broadcom NorthStar plus USB3 PHY driver"
>>>> depends on OF && (ARCH_BCM_NSP || COMPILE_TEST)
>>>> @@ -518,4 +531,5 @@ config PHY_NSP_USB3
>>>> help
>>>> Enable this to support the Broadcom Northstar plus USB3 PHY.
>>>> If unsure, say N.
>>>> +
>>
>> spurious space..
> you're right, this shouldn't be here. I'll fix this in the next version
>
>>>> endmenu
>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>> index e7b0feb1e125..e680b856e38e 100644
>>>> --- a/drivers/phy/Makefile
>>>> +++ b/drivers/phy/Makefile
>>>> @@ -62,4 +62,5 @@ obj-$(CONFIG_PHY_CYGNUS_PCIE) += phy-bcm-cygnus-pcie.o
>>>> obj-$(CONFIG_ARCH_TEGRA) += tegra/
>>>> obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o
>>>> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
>>>> +obj-$(CONFIG_PHY_MESON_GXL_USB) += phy-meson-gxl-usb2.o
>>>> obj-$(CONFIG_PHY_NSP_USB3) += phy-bcm-nsp-usb3.o
>>>> diff --git a/drivers/phy/phy-meson-gxl-usb2.c b/drivers/phy/phy-meson-gxl-usb2.c
>>>> new file mode 100644
>>>> index 000000000000..4bf646a52c45
>>>> --- /dev/null
>>>> +++ b/drivers/phy/phy-meson-gxl-usb2.c
>>>> @@ -0,0 +1,273 @@
>>>> +/*
>>>> + * Meson GXL and GXM USB2 PHY driver
>>>> + *
>>>> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + *
>>>> + * You should have received a copy of the GNU General Public License
>>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>>> + */
>>>> +
>>>> +#include <linux/delay.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/usb/of.h>
>>>> +
>>>> +/* bits [31:27] are read-only */
>>>> +#define U2P_R0 0x0
>>>> + #define U2P_R0_BYPASS_SEL BIT(0)
>>>> + #define U2P_R0_BYPASS_DM_EN BIT(1)
>>>> + #define U2P_R0_BYPASS_DP_EN BIT(2)
>>>> + #define U2P_R0_TXBITSTUFF_ENH BIT(3)
>>>> + #define U2P_R0_TXBITSTUFF_EN BIT(4)
>>>> + #define U2P_R0_DM_PULLDOWN BIT(5)
>>>> + #define U2P_R0_DP_PULLDOWN BIT(6)
>>>> + #define U2P_R0_DP_VBUS_VLD_EXT_SEL BIT(7)
>>>> + #define U2P_R0_DP_VBUS_VLD_EXT BIT(8)
>>>> + #define U2P_R0_ADP_PRB_EN BIT(9)
>>>> + #define U2P_R0_ADP_DISCHARGE BIT(10)
>>>> + #define U2P_R0_ADP_CHARGE BIT(11)
>>>> + #define U2P_R0_DRV_VBUS BIT(12)
>>>> + #define U2P_R0_ID_PULLUP BIT(13)
>>>> + #define U2P_R0_LOOPBACK_EN_B BIT(14)
>>>> + #define U2P_R0_OTG_DISABLE BIT(15)
>>>> + #define U2P_R0_COMMON_ONN BIT(16)
>>>> + #define U2P_R0_FSEL_MASK GENMASK(19, 17)
>>>> + #define U2P_R0_REF_CLK_SEL_MASK GENMASK(21, 20)
>>>> + #define U2P_R0_POWER_ON_RESET BIT(22)
>>>> + #define U2P_R0_V_ATE_TEST_EN_B_MASK GENMASK(24, 23)
>>>> + #define U2P_R0_ID_SET_ID_DQ BIT(25)
>>>> + #define U2P_R0_ATE_RESET BIT(26)
>>>> + #define U2P_R0_FSV_MINUS BIT(27)
>>>> + #define U2P_R0_FSV_PLUS BIT(28)
>>>> + #define U2P_R0_BYPASS_DM_DATA BIT(29)
>>>> + #define U2P_R0_BYPASS_DP_DATA BIT(30)
>>>> +
>>>> +#define U2P_R1 0x4
>>>> + #define U2P_R1_BURN_IN_TEST BIT(0)
>>>> + #define U2P_R1_ACA_ENABLE BIT(1)
>>>> + #define U2P_R1_DCD_ENABLE BIT(2)
>>>> + #define U2P_R1_VDAT_SRC_EN_B BIT(3)
>>>> + #define U2P_R1_VDAT_DET_EN_B BIT(4)
>>>> + #define U2P_R1_CHARGES_SEL BIT(5)
>>>> + #define U2P_R1_TX_PREEMP_PULSE_TUNE BIT(6)
>>>> + #define U2P_R1_TX_PREEMP_AMP_TUNE_MASK GENMASK(8, 7)
>>>> + #define U2P_R1_TX_RES_TUNE_MASK GENMASK(10, 9)
>>>> + #define U2P_R1_TX_RISE_TUNE_MASK GENMASK(12, 11)
>>>> + #define U2P_R1_TX_VREF_TUNE_MASK GENMASK(16, 13)
>>>> + #define U2P_R1_TX_FSLS_TUNE_MASK GENMASK(20, 17)
>>>> + #define U2P_R1_TX_HSXV_TUNE_MASK GENMASK(22, 21)
>>>> + #define U2P_R1_OTG_TUNE_MASK GENMASK(25, 23)
>>>> + #define U2P_R1_SQRX_TUNE_MASK GENMASK(28, 26)
>>>> + #define U2P_R1_COMP_DIS_TUNE_MASK GENMASK(31, 29)
>>>> +
>>>> +/* bits [31:14] are read-only */
>>>> +#define U2P_R2 0x8
>>>> + #define U2P_R2_DATA_IN_MASK GENMASK(3, 0)
>>>> + #define U2P_R2_DATA_IN_EN_MASK GENMASK(7, 4)
>>>> + #define U2P_R2_ADDR_MASK GENMASK(11, 8)
>>>> + #define U2P_R2_DATA_OUT_SEL BIT(12)
>>>> + #define U2P_R2_CLK BIT(13)
>>>> + #define U2P_R2_DATA_OUT_MASK GENMASK(17, 14)
>>>> + #define U2P_R2_ACA_PIN_RANGE_C BIT(18)
>>>> + #define U2P_R2_ACA_PIN_RANGE_B BIT(19)
>>>> + #define U2P_R2_ACA_PIN_RANGE_A BIT(20)
>>>> + #define U2P_R2_ACA_PIN_GND BIT(21)
>>>> + #define U2P_R2_ACA_PIN_FLOAT BIT(22)
>>>> + #define U2P_R2_CHARGE_DETECT BIT(23)
>>>> + #define U2P_R2_DEVICE_SESSION_VALID BIT(24)
>>>> + #define U2P_R2_ADP_PROBE BIT(25)
>>>> + #define U2P_R2_ADP_SENSE BIT(26)
>>>> + #define U2P_R2_SESSION_END BIT(27)
>>>> + #define U2P_R2_VBUS_VALID BIT(28)
>>>> + #define U2P_R2_B_VALID BIT(29)
>>>> + #define U2P_R2_A_VALID BIT(30)
>>>> + #define U2P_R2_ID_DIG BIT(31)
>>>> +
>>>> +#define U2P_R3 0xc
>>>> +
>>>> +#define RESET_COMPLETE_TIME 500
>>>> +
>>>> +struct phy_meson_gxl_usb2_priv {
>>>> + struct regmap *regmap;
>>>> + enum phy_mode mode;
>>>> + int is_enabled;
>>>> +};
>>>> +
>>>> +static const struct regmap_config phy_meson_gxl_usb2_regmap_conf = {
>>>> + .reg_bits = 8,
>>>> + .val_bits = 32,
>>>> + .reg_stride = 4,
>>>> + .max_register = U2P_R3,
>>>> +};
>>>> +
>>>> +static int phy_meson_gxl_usb2_reset(struct phy *phy)
>>>> +{
>>>> + struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
>>>> +
>>>> + if (priv->is_enabled) {
>>>> + /* reset the PHY and wait until settings are stabilized */
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET,
>>>> + U2P_R0_POWER_ON_RESET);
>>>> + udelay(RESET_COMPLETE_TIME);
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET,
>>>> + 0);
>>>> + udelay(RESET_COMPLETE_TIME);
>>>> + }
>>
>> Instead of having big if condition blocks, it could be
>> if (!priv->is_enabled)
>> return 0
>>
>> the configuration when priv->is_enabled is true should go here.
> makes sense (and the code easier to read) in this case - I'll fix it
> in the next version
>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode)
>>>> +{
>>>> + struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
>>>> +
>>>> + switch (mode) {
>>>> + case PHY_MODE_USB_HOST:
>>>> + case PHY_MODE_USB_OTG:
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_DM_PULLDOWN,
>>>> + U2P_R0_DM_PULLDOWN);
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_DP_PULLDOWN,
>>>> + U2P_R0_DP_PULLDOWN);
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_ID_PULLUP, 0);
>>>> + break;
>>>> +
>>>> + case PHY_MODE_USB_DEVICE:
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_DM_PULLDOWN,
>>>> + 0);
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_DP_PULLDOWN,
>>>> + 0);
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_ID_PULLUP,
>>>> + U2P_R0_ID_PULLUP);
>>>> + break;
>>>> +
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + phy_meson_gxl_usb2_reset(phy);
>>>> +
>>>> + priv->mode = mode;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int phy_meson_gxl_usb2_power_off(struct phy *phy)
>>>> +{
>>>> + struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
>>>> +
>>>> + priv->is_enabled = 0;
>>>> +
>>>> + /* power off the PHY by putting it into reset mode */
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET,
>>>> + U2P_R0_POWER_ON_RESET);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int phy_meson_gxl_usb2_power_on(struct phy *phy)
>>>> +{
>>>> + struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
>>>> + int ret;
>>>> +
>>>> + priv->is_enabled = 1;
>>>> +
>>>> + /* power on the PHY by taking it out of reset mode */
>>>> + regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET, 0);
>>>> +
>>>> + ret = phy_meson_gxl_usb2_set_mode(phy, priv->mode);
>>
>> Since this is already part of phy_ops, the consumer of this phy will take care
>> of setting the mode right?
> many USB PHYs are configured through of_usb_get_dr_mode_by_phy()
> (which fetches the USB controller DT node and looks up the dr_mode
> property). this means that the driver will even work for controller
> which did not call phy_set_mode() (currently dwc2 and dwc3 are an
> example where phy_set_mode() is not called, and this driver is
> probably going to be used by both). are you fine with keeping this or
> should we instead investigate why dwc2 and dwc3 are not calling
> phy_set_mode yet?
If the driver want to set_mode of the phy, then it should be fine. But let's
remove .set_mode from phy_ops. Either the driver should set the mode by itself
or allow the consumers to set the mode but not both.
>
>>>> + if (ret) {
>>>> + phy_meson_gxl_usb2_power_off(phy);
>>
>> This would mess up the reference count in phy_core, since this function is also
>> part of the phy_ops. The consumer should be responsible for powering off the phy.
> actually it shouldn't. it would cause issues if I used
> phy_power_off(phy) here, but I'm using the driver-internal callback
> here.
I didn't mean you should use phy_power_off(). I meant powering on the phy or
powering off the phy should be controlled by the consumer.
> if phy_meson_gxl_usb2_power_on() fails then phy_power_on() (from
> phy-core) prints a warnings and does not increment the internal
> ref-counter (++phy->power_count).
> so in my opinion there shouldn't be a problem
That makes sense.
Thanks
Kishon
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] dt-bindings: update the Allwinner GPADC device tree binding for H3
From: Maxime Ripard @ 2017-04-04 13:20 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Lee Jones, Chen-Yu Tsai, Jonathan Cameron, Quentin Schulz,
Zhang Rui, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <e4d50e78-afe7-6107-6d9a-15e4c732d79a-h8G6r0blFSE@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
On Mon, Apr 03, 2017 at 05:31:11PM +0800, Icenowy Zheng wrote:
>
>
> 在 2017年04月03日 17:15, Maxime Ripard 写道:
> > On Sun, Apr 02, 2017 at 09:33:01PM +0800, Icenowy Zheng wrote:
> > > Allwinner H3 features a thermal sensor like the one in A33, but has its
> > > register re-arranged, the clock divider moved to CCU (originally the
> > > clock divider is in ADC) and added a pair of bus clock and reset.
> > >
> > > Update the binding document to cover H3.
> > >
> > > Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> > > ---
> > > .../devicetree/bindings/mfd/sun4i-gpadc.txt | 23 ++++++++++++++++++++--
> > > 1 file changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > index badff3611a98..7753133ca0ff 100644
> > > --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > @@ -4,12 +4,20 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
> > > and sometimes as a touchscreen controller.
> > >
> > > Required properties:
> > > - - compatible: "allwinner,sun8i-a33-ths",
> > > + - compatible: must contain one of the following compatibles:
> > > + - "allwinner,sun8i-a33-ths"
> > > + - "allwinner,sun8i-h3-ths"
> > > - reg: mmio address range of the chip,
> > > - #thermal-sensor-cells: shall be 0,
> > > - #io-channel-cells: shall be 0,
> > >
> > > -Example:
> > > +Required properties for the following compatibles:
> > > + - "allwinner,sun8i-h3-ths"
> > > + - clocks: the bus clock and the input clock of the ADC,
> > > + - clock-names: should be "bus" and "ths",
> >
> > I guess mod instead of ths would be more consistent.
>
> In fact I will prefer "sample" here if not "ths", as it's used
> for the sampling, not like other mod clocks, which controls
> all the IP block's logic.
All the other mod clocks control some sort of sampling. The audio mod
clocks will be used to sample data in capture, same thing for NAND,
MMC, SPI, etc.
Please remain consistent.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH 2/2] clk: vc5: Add support for IDT VersaClock 5P49V5935
From: Alexey Firago @ 2017-04-04 13:16 UTC (permalink / raw)
To: mturquette, sboyd, robh+dt, marek.vasut, linux-clk, devicetree
Cc: Alexey Firago
In-Reply-To: <1491311788-31905-1-git-send-email-alexey_firago@mentor.com>
Update IDT VersaClock 5 driver to support 5P49V5935. This chip has
two clock inputs (internal XTAL or external CLKIN), four fractional
dividers (FODs) and five clock outputs (four universal clock outputs
and one reference clock output at OUT0_SELB_I2C).
Current driver supports up to 2 FODs and up to 3 clock outputs.
This patch sets max number of supported FODs to 4 and max number
of supported clocks to 5.
Number of used FODs and clock outputs is set on probe according to
the model specified via device-tree.
Signed-off-by: Alexey Firago <alexey_firago@mentor.com>
---
drivers/clk/clk-versaclock5.c | 50 ++++++++++++++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index 56741f3..f74d6e4 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -113,10 +113,17 @@
#define VC5_MUX_IN_XIN BIT(0)
#define VC5_MUX_IN_CLKIN BIT(1)
+/* Maximum number of clk_out supported by this driver */
+#define VC5_MAX_CLK_OUT_NUM 5
+
+/* Maximum number of FODs supported by this driver */
+#define VC5_MAX_FOD_NUM 4
+
/* Supported IDT VC5 models. */
enum vc5_model {
IDT_VC5_5P49V5923,
IDT_VC5_5P49V5933,
+ IDT_VC5_5P49V5935,
};
struct vc5_driver_data;
@@ -139,8 +146,10 @@ struct vc5_driver_data {
unsigned char clk_mux_ins;
struct clk_hw clk_mux;
struct vc5_hw_data clk_pll;
- struct vc5_hw_data clk_fod[2];
- struct vc5_hw_data clk_out[3];
+ int clk_fod_cnt;
+ struct vc5_hw_data clk_fod[VC5_MAX_FOD_NUM];
+ int clk_out_cnt;
+ struct vc5_hw_data clk_out[VC5_MAX_CLK_OUT_NUM];
};
static const char * const vc5_mux_names[] = {
@@ -563,7 +572,7 @@ static struct clk_hw *vc5_of_clk_get(struct of_phandle_args *clkspec,
struct vc5_driver_data *vc5 = data;
unsigned int idx = clkspec->args[0];
- if (idx > 2)
+ if (idx > vc5->clk_out_cnt)
return ERR_PTR(-EINVAL);
return &vc5->clk_out[idx].hw;
@@ -576,6 +585,7 @@ static int vc5_map_index_to_output(const enum vc5_model model,
case IDT_VC5_5P49V5933:
return (n == 0) ? 0 : 3;
case IDT_VC5_5P49V5923:
+ case IDT_VC5_5P49V5935:
default:
return n;
}
@@ -591,7 +601,7 @@ static int vc5_probe(struct i2c_client *client,
struct vc5_driver_data *vc5;
struct clk_init_data init;
const char *parent_names[2];
- unsigned int n, idx;
+ unsigned int n, idx = 0;
int ret;
vc5 = devm_kzalloc(&client->dev, sizeof(*vc5), GFP_KERNEL);
@@ -602,6 +612,23 @@ static int vc5_probe(struct i2c_client *client,
vc5->client = client;
vc5->model = (enum vc5_model)of_id->data;
+ /* Set number of supported outputs according to the repoted model */
+ switch (vc5->model) {
+ case IDT_VC5_5P49V5923:
+ case IDT_VC5_5P49V5933:
+ vc5->clk_fod_cnt = 2;
+ vc5->clk_out_cnt = 3;
+ break;
+ case IDT_VC5_5P49V5935:
+ vc5->clk_fod_cnt = 4;
+ vc5->clk_out_cnt = 5;
+ break;
+ default:
+ /* Should never go here */
+ dev_err(&client->dev, "unsupported IDT VC5 ID specified\n");
+ return -EINVAL;
+ }
+
vc5->pin_xin = devm_clk_get(&client->dev, "xin");
if (PTR_ERR(vc5->pin_xin) == -EPROBE_DEFER)
return -EPROBE_DEFER;
@@ -622,8 +649,9 @@ static int vc5_probe(struct i2c_client *client,
if (!IS_ERR(vc5->pin_xin)) {
vc5->clk_mux_ins |= VC5_MUX_IN_XIN;
parent_names[init.num_parents++] = __clk_get_name(vc5->pin_xin);
- } else if (vc5->model == IDT_VC5_5P49V5933) {
- /* IDT VC5 5P49V5933 has built-in oscilator. */
+ } else if (vc5->model == IDT_VC5_5P49V5933 ||
+ vc5->model == IDT_VC5_5P49V5935) {
+ /* IDT VC5 5P49V5933 and 5P49V5935 have built-in oscilator. */
vc5->pin_xin = clk_register_fixed_rate(&client->dev,
"internal-xtal", NULL,
0, 25000000);
@@ -672,7 +700,7 @@ static int vc5_probe(struct i2c_client *client,
}
/* Register FODs */
- for (n = 0; n < 2; n++) {
+ for (n = 0; n < vc5->clk_fod_cnt; n++) {
idx = vc5_map_index_to_output(vc5->model, n);
memset(&init, 0, sizeof(init));
init.name = vc5_fod_names[idx];
@@ -709,7 +737,7 @@ static int vc5_probe(struct i2c_client *client,
}
/* Register FOD-connected OUTx outputs */
- for (n = 1; n < 3; n++) {
+ for (n = 1; n < vc5->clk_out_cnt; n++) {
idx = vc5_map_index_to_output(vc5->model, n - 1);
parent_names[0] = vc5_fod_names[idx];
if (n == 1)
@@ -744,7 +772,7 @@ static int vc5_probe(struct i2c_client *client,
return 0;
err_clk:
- if (vc5->model == IDT_VC5_5P49V5933)
+ if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
clk_unregister_fixed_rate(vc5->pin_xin);
return ret;
}
@@ -755,7 +783,7 @@ static int vc5_remove(struct i2c_client *client)
of_clk_del_provider(client->dev.of_node);
- if (vc5->model == IDT_VC5_5P49V5933)
+ if (vc5->model == IDT_VC5_5P49V5933 || vc5->model == IDT_VC5_5P49V5935)
clk_unregister_fixed_rate(vc5->pin_xin);
return 0;
@@ -764,6 +792,7 @@ static int vc5_remove(struct i2c_client *client)
static const struct i2c_device_id vc5_id[] = {
{ "5p49v5923", .driver_data = IDT_VC5_5P49V5923 },
{ "5p49v5933", .driver_data = IDT_VC5_5P49V5933 },
+ { "5p49v5935", .driver_data = IDT_VC5_5P49V5935 },
{ }
};
MODULE_DEVICE_TABLE(i2c, vc5_id);
@@ -771,6 +800,7 @@ MODULE_DEVICE_TABLE(i2c, vc5_id);
static const struct of_device_id clk_vc5_of_match[] = {
{ .compatible = "idt,5p49v5923", .data = (void *)IDT_VC5_5P49V5923 },
{ .compatible = "idt,5p49v5933", .data = (void *)IDT_VC5_5P49V5933 },
+ { .compatible = "idt,5p49v5935", .data = (void *)IDT_VC5_5P49V5935 },
{ },
};
MODULE_DEVICE_TABLE(of, clk_vc5_of_match);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] clk: vc5: Add bindings for IDT VersaClock 5P49V5935
From: Alexey Firago @ 2017-04-04 13:16 UTC (permalink / raw)
To: mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Alexey Firago
In-Reply-To: <1491311788-31905-1-git-send-email-alexey_firago-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
IDT VersaClock 5 5P49V5935 has 4 clock outputs, 4 fractional dividers.
Input clock source can be taken from either integrated crystal or from
external reference clock.
Signed-off-by: Alexey Firago <alexey_firago-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/clock/idt,versaclock5.txt | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
index 87e9c47..53d7e50 100644
--- a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
@@ -6,18 +6,21 @@ from 3 to 12 output clocks.
==I2C device node==
Required properties:
-- compatible: shall be one of "idt,5p49v5923" , "idt,5p49v5933".
+- compatible: shall be one of "idt,5p49v5923" , "idt,5p49v5933" ,
+ "idt,5p49v5935".
- reg: i2c device address, shall be 0x68 or 0x6a.
- #clock-cells: from common clock binding; shall be set to 1.
- clocks: from common clock binding; list of parent clock handles,
- 5p49v5923: (required) either or both of XTAL or CLKIN
reference clock.
- - 5p49v5933: (optional) property not present (internal
+ - 5p49v5933 and
+ - 5p49v5935: (optional) property not present (internal
Xtal used) or CLKIN reference
clock.
- clock-names: from common clock binding; clock input names, can be
- 5p49v5923: (required) either or both of "xin", "clkin".
- - 5p49v5933: (optional) property not present or "clkin".
+ - 5p49v5933 and
+ - 5p49v5935: (optional) property not present or "clkin".
==Mapping between clock specifier and physical pins==
@@ -34,6 +37,13 @@ clock specifier, the following mapping applies:
1 -- OUT1
2 -- OUT4
+5P49V5935:
+ 0 -- OUT0_SEL_I2CB
+ 1 -- OUT1
+ 2 -- OUT2
+ 3 -- OUT3
+ 4 -- OUT4
+
==Example==
/* 25MHz reference crystal */
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] clk: Add support for IDT 5P49V5935
From: Alexey Firago @ 2017-04-04 13:16 UTC (permalink / raw)
To: mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Alexey Firago
This series adds support for IDT VersaClock 5P49V5935 programmable clock
generator to the existing clk-versaclock5 driver.
Patches were verified on Avnet UltraZed-EG board with IO Carrier Card.
Alexey Firago (2):
clk: vc5: Add bindings for IDT VersaClock 5P49V5935
clk: vc5: Add support for IDT VersaClock 5P49V5935
.../devicetree/bindings/clock/idt,versaclock5.txt | 16 +++++--
drivers/clk/clk-versaclock5.c | 50 +++++++++++++++++-----
2 files changed, 53 insertions(+), 13 deletions(-)
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] dt-bindings: display: rk3288-mipi-dsi: add reset property
From: John Keeping @ 2017-04-04 13:15 UTC (permalink / raw)
To: Sean Paul
Cc: Rob Herring, Brian Norris, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chris Zhong,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170312120659.hpiwnsto6hkxs5yh@rob-hp-laptop>
Hi Sean,
On Sun, 12 Mar 2017 07:06:59 -0500, Rob Herring wrote:
> On Fri, Mar 03, 2017 at 11:39:45AM +0000, John Keeping wrote:
> > This reset is required in order to fully reset the internal state of the
> > MIPI controller.
> >
> > Signed-off-by: John Keeping <john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>
> > ---
> > On Thu, 2 Mar 2017 13:56:46 -0800, Brian Norris wrote:
> > > On Fri, Feb 24, 2017 at 12:55:06PM +0000, John Keeping wrote:
> > > > + /*
> > > > + * Note that the reset was not defined in the initial device tree, so
> > > > + * we have to be prepared for it not being found.
> > > > + */
> > > > + apb_rst = devm_reset_control_get(dev, "apb");
> > >
> > > Did this reset ever get documented in the device tree bindings? I
> > > couldn't find it. Perhaps a follow-up patch is in order?
> >
> > Here's a patch to do that.
> >
> > .../devicetree/bindings/display/rockchip/dw_mipi_dsi_rockchip.txt | 7 +++++++
> > 1 file changed, 7 insertions(+)
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
I don't see this patch in linux-next, so I guess it has fallen through
the cracks somewhere. Since we have Rob's ack, can you pick this via
drm-misc?
Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2 v2] ARM: i.MX25: globally disable supervisor protect
From: Shawn Guo @ 2017-04-04 13:14 UTC (permalink / raw)
To: Martin Kaiser
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491248825-23041-2-git-send-email-martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
On Mon, Apr 03, 2017 at 09:47:05PM +0200, Martin Kaiser wrote:
> The problem described in 6befda9a272b98bfb1dc772efc3564644cbfb270
This causes the following checkpatch error.
ERROR: Please use git commit description style 'commit <12+ chars of
sha1> ("<title line>")' - ie: 'commit 6befda9a272b ("ARM: i.MX53:
globally disable supervisor protect")'
I fixed it and applied the patch.
Shawn
> for the i.MX53 platform applies to i.MX25 as well.
>
> E.g. CSPI1+SDMA and SSI1+SDMA are not working with the default AIPS
> configuration. Modifiy the AIPS configuration to allow access to the bus
> by SDMA and peripherals.
>
> Signed-off-by: Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
> ---
> v2:
> rebased against latest linux-next
> re-sending both patches, it seems they got lost along the way
>
> arch/arm/mach-imx/mach-imx25.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/mach-imx/mach-imx25.c b/arch/arm/mach-imx/mach-imx25.c
> index 32dcb5e..353b86e 100644
> --- a/arch/arm/mach-imx/mach-imx25.c
> +++ b/arch/arm/mach-imx/mach-imx25.c
> @@ -23,6 +23,11 @@ static void __init imx25_init_early(void)
> mxc_set_cpu_type(MXC_CPU_MX25);
> }
>
> +static void __init imx25_dt_init(void)
> +{
> + imx_aips_allow_unprivileged_access("fsl,imx25-aips");
> +}
> +
> static void __init mx25_init_irq(void)
> {
> struct device_node *np;
> @@ -41,6 +46,7 @@ static const char * const imx25_dt_board_compat[] __initconst = {
>
> DT_MACHINE_START(IMX25_DT, "Freescale i.MX25 (Device Tree Support)")
> .init_early = imx25_init_early,
> + .init_machine = imx25_dt_init,
> .init_late = imx25_pm_init,
> .init_irq = mx25_init_irq,
> .dt_compat = imx25_dt_board_compat,
> --
> 2.1.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2 v4] ARM: dts: i.MX25: add AIPS control registers
From: Shawn Guo @ 2017-04-04 13:13 UTC (permalink / raw)
To: Martin Kaiser
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sascha Hauer,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491248825-23041-1-git-send-email-martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
On Mon, Apr 03, 2017 at 09:47:04PM +0200, Martin Kaiser wrote:
> The i.MX25 contains two AHB to IP bridges (AIPS), each of which has a set of
> control registers. Add the memory regions for the control registers to
> the Device Tree.
>
> Signed-off-by: Martin Kaiser <martin-XxZfDwE/svGeZLLa646FqQ@public.gmane.org>
> Reviewed-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] Documentation: DT: Add entry for FSL LS1088A RDB, QDS boards
From: Shawn Guo @ 2017-04-04 12:59 UTC (permalink / raw)
To: Harninder Rai
Cc: Rob Herring, oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stuart Yoder,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <VI1PR0402MB2863D1744E831A8B300580D3E65A0-9IDQY6o3qQh11F+3UnuzFY3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Thu, Feb 16, 2017 at 07:44:51AM +0000, Harninder Rai wrote:
> >
> > Some consistency in the subject please "dt-bindings: arm: ..."
> Thanks Rob.
>
> Hello Shawn,
>
> Would you please take care of the subject line while applying the patch or should I send another revision?
Applied 1/3 and this one with subject fixed as suggested.
Shawn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH linux v6 2/2] drivers: hwmon: Support for ASPEED PWM/Fan tach
From: Guenter Roeck @ 2017-04-04 12:55 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan, joel, jdelvare,
linux-hwmon, linux-kernel, openbmc, corbet, linux-doc, robh+dt,
mark.rutland, devicetree
In-Reply-To: <20170403233025.26952-3-jaghu@google.com>
On 04/03/2017 04:30 PM, Jaghathiswari Rankappagounder Natarajan wrote:
> The ASPEED AST2400/2500 PWM controller supports 8 PWM output ports.
> The ASPEED AST2400/2500 Fan tach controller supports 16 tachometer
> inputs.
> The device driver matches on the device tree node. The configuration
> values are read from the device tree and written to the respective registers.
> The driver provides a sysfs entries through which the user can
> configure the duty-cycle value (ranging from 0 to 100 percent) and read
> the fan tach rpm value.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
Applied.
Thanks,
Guenter
> ---
> v6
> - Corrected odd line breaks
> - Changed upto to up to
> - Dropped unrelated changes
> - Removed struct and used regs pointer directly
> - Made groups to be null terminated
> - Made correction in calculation of val/raw_data
> - Removed else after return
> - Removed unnecessary continuation lines
>
> v5:
> - Changed the driver to suit the changes in the device tree documentation
>
> v4:
> - Modified this driver to suit the representation in the devicetree
>
> v3:
> - Only sent out device tree documentation; did not send this driver
>
> v2:
> - Used BIT()
> - Used regmap
> - Avoided division when raw data is 0
> - Removed empty lines between declaration
> - Removed macros; Used two attribute groups and used is_visible callback
> - Returned error when properties are undefined
> - Removed .owner field
> - Used PTR_ERR_OR_ZERO
> - Removed explicit of_node_put for child nodes
>
> Documentation/hwmon/aspeed-pwm-tacho | 22 +
> drivers/hwmon/Kconfig | 9 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/aspeed-pwm-tacho.c | 846 +++++++++++++++++++++++++++++++++++
> 4 files changed, 878 insertions(+)
> create mode 100644 Documentation/hwmon/aspeed-pwm-tacho
> create mode 100644 drivers/hwmon/aspeed-pwm-tacho.c
>
> diff --git a/Documentation/hwmon/aspeed-pwm-tacho b/Documentation/hwmon/aspeed-pwm-tacho
> new file mode 100644
> index 000000000000..7cfb34977460
> --- /dev/null
> +++ b/Documentation/hwmon/aspeed-pwm-tacho
> @@ -0,0 +1,22 @@
> +Kernel driver aspeed-pwm-tacho
> +==============================
> +
> +Supported chips:
> + ASPEED AST2400/2500
> +
> +Authors:
> + <jaghu@google.com>
> +
> +Description:
> +------------
> +This driver implements support for ASPEED AST2400/2500 PWM and Fan Tacho
> +controller. The PWM controller supports upto 8 PWM outputs. The Fan tacho
> +controller supports up to 16 tachometer inputs.
> +
> +The driver provides the following sensor accesses in sysfs:
> +
> +fanX_input ro provide current fan rotation value in RPM as reported
> + by the fan to the device.
> +
> +pwmX rw get or set PWM fan control value. This is an integer
> + value between 0(off) and 255(full speed).
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 45cef3d2c75c..757b5b0705bf 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -341,6 +341,15 @@ config SENSORS_ASB100
> This driver can also be built as a module. If so, the module
> will be called asb100.
>
> +config SENSORS_ASPEED
> + tristate "ASPEED AST2400/AST2500 PWM and Fan tach driver"
> + help
> + This driver provides support for ASPEED AST2400/AST2500 PWM
> + and Fan Tacho controllers.
> +
> + This driver can also be built as a module. If so, the module
> + will be called aspeed_pwm_tacho.
> +
> config SENSORS_ATXP1
> tristate "Attansic ATXP1 VID controller"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index aecf4ba17460..83025cc9bb45 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
> obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
> obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
> obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
> +obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
> obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
> obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
> new file mode 100644
> index 000000000000..29010ad94208
> --- /dev/null
> +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> @@ -0,0 +1,846 @@
> +/*
> + * Copyright (c) 2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or later as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/sysfs.h>
> +#include <linux/regmap.h>
> +
> +/* ASPEED PWM & FAN Tach Register Definition */
> +#define ASPEED_PTCR_CTRL 0x00
> +#define ASPEED_PTCR_CLK_CTRL 0x04
> +#define ASPEED_PTCR_DUTY0_CTRL 0x08
> +#define ASPEED_PTCR_DUTY1_CTRL 0x0c
> +#define ASPEED_PTCR_TYPEM_CTRL 0x10
> +#define ASPEED_PTCR_TYPEM_CTRL1 0x14
> +#define ASPEED_PTCR_TYPEN_CTRL 0x18
> +#define ASPEED_PTCR_TYPEN_CTRL1 0x1c
> +#define ASPEED_PTCR_TACH_SOURCE 0x20
> +#define ASPEED_PTCR_TRIGGER 0x28
> +#define ASPEED_PTCR_RESULT 0x2c
> +#define ASPEED_PTCR_INTR_CTRL 0x30
> +#define ASPEED_PTCR_INTR_STS 0x34
> +#define ASPEED_PTCR_TYPEM_LIMIT 0x38
> +#define ASPEED_PTCR_TYPEN_LIMIT 0x3C
> +#define ASPEED_PTCR_CTRL_EXT 0x40
> +#define ASPEED_PTCR_CLK_CTRL_EXT 0x44
> +#define ASPEED_PTCR_DUTY2_CTRL 0x48
> +#define ASPEED_PTCR_DUTY3_CTRL 0x4c
> +#define ASPEED_PTCR_TYPEO_CTRL 0x50
> +#define ASPEED_PTCR_TYPEO_CTRL1 0x54
> +#define ASPEED_PTCR_TACH_SOURCE_EXT 0x60
> +#define ASPEED_PTCR_TYPEO_LIMIT 0x78
> +
> +/* ASPEED_PTCR_CTRL : 0x00 - General Control Register */
> +#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1 15
> +#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2 6
> +#define ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK (BIT(7) | BIT(15))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1 14
> +#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2 5
> +#define ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK (BIT(6) | BIT(14))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1 13
> +#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2 4
> +#define ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK (BIT(5) | BIT(13))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1 12
> +#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2 3
> +#define ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK (BIT(4) | BIT(12))
> +
> +#define ASPEED_PTCR_CTRL_FAN_NUM_EN(x) BIT(16 + (x))
> +
> +#define ASPEED_PTCR_CTRL_PWMD_EN BIT(11)
> +#define ASPEED_PTCR_CTRL_PWMC_EN BIT(10)
> +#define ASPEED_PTCR_CTRL_PWMB_EN BIT(9)
> +#define ASPEED_PTCR_CTRL_PWMA_EN BIT(8)
> +
> +#define ASPEED_PTCR_CTRL_CLK_SRC BIT(1)
> +#define ASPEED_PTCR_CTRL_CLK_EN BIT(0)
> +
> +/* ASPEED_PTCR_CLK_CTRL : 0x04 - Clock Control Register */
> +/* TYPE N */
> +#define ASPEED_PTCR_CLK_CTRL_TYPEN_MASK GENMASK(31, 16)
> +#define ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT 24
> +#define ASPEED_PTCR_CLK_CTRL_TYPEN_H 20
> +#define ASPEED_PTCR_CLK_CTRL_TYPEN_L 16
> +/* TYPE M */
> +#define ASPEED_PTCR_CLK_CTRL_TYPEM_MASK GENMASK(15, 0)
> +#define ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT 8
> +#define ASPEED_PTCR_CLK_CTRL_TYPEM_H 4
> +#define ASPEED_PTCR_CLK_CTRL_TYPEM_L 0
> +
> +/*
> + * ASPEED_PTCR_DUTY_CTRL/1/2/3 : 0x08/0x0C/0x48/0x4C - PWM-FAN duty control
> + * 0/1/2/3 register
> + */
> +#define DUTY_CTRL_PWM2_FALL_POINT 24
> +#define DUTY_CTRL_PWM2_RISE_POINT 16
> +#define DUTY_CTRL_PWM2_RISE_FALL_MASK GENMASK(31, 16)
> +#define DUTY_CTRL_PWM1_FALL_POINT 8
> +#define DUTY_CTRL_PWM1_RISE_POINT 0
> +#define DUTY_CTRL_PWM1_RISE_FALL_MASK GENMASK(15, 0)
> +
> +/* ASPEED_PTCR_TYPEM_CTRL : 0x10/0x18/0x50 - Type M/N/O Ctrl 0 Register */
> +#define TYPE_CTRL_FAN_MASK (GENMASK(5, 1) | GENMASK(31, 16))
> +#define TYPE_CTRL_FAN1_MASK GENMASK(31, 0)
> +#define TYPE_CTRL_FAN_PERIOD 16
> +#define TYPE_CTRL_FAN_MODE 4
> +#define TYPE_CTRL_FAN_DIVISION 1
> +#define TYPE_CTRL_FAN_TYPE_EN 1
> +
> +/* ASPEED_PTCR_TACH_SOURCE : 0x20/0x60 - Tach Source Register */
> +/* bit [0,1] at 0x20, bit [2] at 0x60 */
> +#define TACH_PWM_SOURCE_BIT01(x) ((x) * 2)
> +#define TACH_PWM_SOURCE_BIT2(x) ((x) * 2)
> +#define TACH_PWM_SOURCE_MASK_BIT01(x) (0x3 << ((x) * 2))
> +#define TACH_PWM_SOURCE_MASK_BIT2(x) BIT((x) * 2)
> +
> +/* ASPEED_PTCR_RESULT : 0x2c - Result Register */
> +#define RESULT_STATUS_MASK BIT(31)
> +#define RESULT_VALUE_MASK 0xfffff
> +
> +/* ASPEED_PTCR_CTRL_EXT : 0x40 - General Control Extension #1 Register */
> +#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1 15
> +#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2 6
> +#define ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK (BIT(7) | BIT(15))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1 14
> +#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2 5
> +#define ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK (BIT(6) | BIT(14))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1 13
> +#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2 4
> +#define ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK (BIT(5) | BIT(13))
> +
> +#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1 12
> +#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2 3
> +#define ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK (BIT(4) | BIT(12))
> +
> +#define ASPEED_PTCR_CTRL_PWMH_EN BIT(11)
> +#define ASPEED_PTCR_CTRL_PWMG_EN BIT(10)
> +#define ASPEED_PTCR_CTRL_PWMF_EN BIT(9)
> +#define ASPEED_PTCR_CTRL_PWME_EN BIT(8)
> +
> +/* ASPEED_PTCR_CLK_EXT_CTRL : 0x44 - Clock Control Extension #1 Register */
> +/* TYPE O */
> +#define ASPEED_PTCR_CLK_CTRL_TYPEO_MASK GENMASK(15, 0)
> +#define ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT 8
> +#define ASPEED_PTCR_CLK_CTRL_TYPEO_H 4
> +#define ASPEED_PTCR_CLK_CTRL_TYPEO_L 0
> +
> +#define PWM_MAX 255
> +
> +#define M_PWM_DIV_H 0x00
> +#define M_PWM_DIV_L 0x05
> +#define M_PWM_PERIOD 0x5F
> +#define M_TACH_CLK_DIV 0x00
> +#define M_TACH_MODE 0x00
> +#define M_TACH_UNIT 0x1000
> +#define INIT_FAN_CTRL 0xFF
> +
> +void __iomem *regs;
> +
> +struct aspeed_pwm_tacho_data {
> + struct regmap *regmap;
> + unsigned long clk_freq;
> + bool pwm_present[8];
> + bool fan_tach_present[16];
> + u8 type_pwm_clock_unit[3];
> + u8 type_pwm_clock_division_h[3];
> + u8 type_pwm_clock_division_l[3];
> + u8 type_fan_tach_clock_division[3];
> + u16 type_fan_tach_unit[3];
> + u8 pwm_port_type[8];
> + u8 pwm_port_fan_ctrl[8];
> + u8 fan_tach_ch_source[16];
> + const struct attribute_group *groups[3];
> +};
> +
> +enum type { TYPEM, TYPEN, TYPEO };
> +
> +struct type_params {
> + u32 l_value;
> + u32 h_value;
> + u32 unit_value;
> + u32 clk_ctrl_mask;
> + u32 clk_ctrl_reg;
> + u32 ctrl_reg;
> + u32 ctrl_reg1;
> +};
> +
> +static const struct type_params type_params[] = {
> + [TYPEM] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEM_L,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEM_H,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEM_UNIT,
> + .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEM_MASK,
> + .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
> + .ctrl_reg = ASPEED_PTCR_TYPEM_CTRL,
> + .ctrl_reg1 = ASPEED_PTCR_TYPEM_CTRL1,
> + },
> + [TYPEN] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEN_L,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEN_H,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEN_UNIT,
> + .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEN_MASK,
> + .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL,
> + .ctrl_reg = ASPEED_PTCR_TYPEN_CTRL,
> + .ctrl_reg1 = ASPEED_PTCR_TYPEN_CTRL1,
> + },
> + [TYPEO] = {
> + .l_value = ASPEED_PTCR_CLK_CTRL_TYPEO_L,
> + .h_value = ASPEED_PTCR_CLK_CTRL_TYPEO_H,
> + .unit_value = ASPEED_PTCR_CLK_CTRL_TYPEO_UNIT,
> + .clk_ctrl_mask = ASPEED_PTCR_CLK_CTRL_TYPEO_MASK,
> + .clk_ctrl_reg = ASPEED_PTCR_CLK_CTRL_EXT,
> + .ctrl_reg = ASPEED_PTCR_TYPEO_CTRL,
> + .ctrl_reg1 = ASPEED_PTCR_TYPEO_CTRL1,
> + }
> +};
> +
> +enum pwm_port { PWMA, PWMB, PWMC, PWMD, PWME, PWMF, PWMG, PWMH };
> +
> +struct pwm_port_params {
> + u32 pwm_en;
> + u32 ctrl_reg;
> + u32 type_part1;
> + u32 type_part2;
> + u32 type_mask;
> + u32 duty_ctrl_rise_point;
> + u32 duty_ctrl_fall_point;
> + u32 duty_ctrl_reg;
> + u32 duty_ctrl_rise_fall_mask;
> +};
> +
> +static const struct pwm_port_params pwm_port_params[] = {
> + [PWMA] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMA_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMA_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
> + },
> + [PWMB] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMB_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMB_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY0_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
> + },
> + [PWMC] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMC_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMC_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
> + },
> + [PWMD] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMD_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMD_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY1_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
> + },
> + [PWME] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWME_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWME_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWME_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
> + },
> + [PWMF] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMF_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMF_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY2_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
> + },
> + [PWMG] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMG_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMG_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM1_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM1_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM1_RISE_FALL_MASK,
> + },
> + [PWMH] = {
> + .pwm_en = ASPEED_PTCR_CTRL_PWMH_EN,
> + .ctrl_reg = ASPEED_PTCR_CTRL_EXT,
> + .type_part1 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART1,
> + .type_part2 = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_PART2,
> + .type_mask = ASPEED_PTCR_CTRL_SET_PWMH_TYPE_MASK,
> + .duty_ctrl_rise_point = DUTY_CTRL_PWM2_RISE_POINT,
> + .duty_ctrl_fall_point = DUTY_CTRL_PWM2_FALL_POINT,
> + .duty_ctrl_reg = ASPEED_PTCR_DUTY3_CTRL,
> + .duty_ctrl_rise_fall_mask = DUTY_CTRL_PWM2_RISE_FALL_MASK,
> + }
> +};
> +
> +static int regmap_aspeed_pwm_tacho_reg_write(void *context, unsigned int reg,
> + unsigned int val)
> +{
> + void __iomem *regs = context;
> +
> + writel(val, regs + reg);
> + return 0;
> +}
> +
> +static int regmap_aspeed_pwm_tacho_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + void __iomem *regs = context;
> +
> + *val = readl(regs + reg);
> + return 0;
> +}
> +
> +static const struct regmap_config aspeed_pwm_tacho_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = ASPEED_PTCR_TYPEO_LIMIT,
> + .reg_write = regmap_aspeed_pwm_tacho_reg_write,
> + .reg_read = regmap_aspeed_pwm_tacho_reg_read,
> + .fast_io = true,
> +};
> +
> +static void aspeed_set_clock_enable(struct regmap *regmap, bool val)
> +{
> + regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
> + ASPEED_PTCR_CTRL_CLK_EN,
> + val ? ASPEED_PTCR_CTRL_CLK_EN : 0);
> +}
> +
> +static void aspeed_set_clock_source(struct regmap *regmap, int val)
> +{
> + regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
> + ASPEED_PTCR_CTRL_CLK_SRC,
> + val ? ASPEED_PTCR_CTRL_CLK_SRC : 0);
> +}
> +
> +static void aspeed_set_pwm_clock_values(struct regmap *regmap, u8 type,
> + u8 div_high, u8 div_low, u8 unit)
> +{
> + u32 reg_value = ((div_high << type_params[type].h_value) |
> + (div_low << type_params[type].l_value) |
> + (unit << type_params[type].unit_value));
> +
> + regmap_update_bits(regmap, type_params[type].clk_ctrl_reg,
> + type_params[type].clk_ctrl_mask, reg_value);
> +}
> +
> +static void aspeed_set_pwm_port_enable(struct regmap *regmap, u8 pwm_port,
> + bool enable)
> +{
> + regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
> + pwm_port_params[pwm_port].pwm_en,
> + enable ? pwm_port_params[pwm_port].pwm_en : 0);
> +}
> +
> +static void aspeed_set_pwm_port_type(struct regmap *regmap,
> + u8 pwm_port, u8 type)
> +{
> + u32 reg_value = (type & 0x1) << pwm_port_params[pwm_port].type_part1;
> +
> + reg_value |= (type & 0x2) << pwm_port_params[pwm_port].type_part2;
> +
> + regmap_update_bits(regmap, pwm_port_params[pwm_port].ctrl_reg,
> + pwm_port_params[pwm_port].type_mask, reg_value);
> +}
> +
> +static void aspeed_set_pwm_port_duty_rising_falling(struct regmap *regmap,
> + u8 pwm_port, u8 rising,
> + u8 falling)
> +{
> + u32 reg_value = (rising <<
> + pwm_port_params[pwm_port].duty_ctrl_rise_point);
> + reg_value |= (falling <<
> + pwm_port_params[pwm_port].duty_ctrl_fall_point);
> +
> + regmap_update_bits(regmap, pwm_port_params[pwm_port].duty_ctrl_reg,
> + pwm_port_params[pwm_port].duty_ctrl_rise_fall_mask,
> + reg_value);
> +}
> +
> +static void aspeed_set_tacho_type_enable(struct regmap *regmap, u8 type,
> + bool enable)
> +{
> + regmap_update_bits(regmap, type_params[type].ctrl_reg,
> + TYPE_CTRL_FAN_TYPE_EN,
> + enable ? TYPE_CTRL_FAN_TYPE_EN : 0);
> +}
> +
> +static void aspeed_set_tacho_type_values(struct regmap *regmap, u8 type,
> + u8 mode, u16 unit, u8 division)
> +{
> + u32 reg_value = ((mode << TYPE_CTRL_FAN_MODE) |
> + (unit << TYPE_CTRL_FAN_PERIOD) |
> + (division << TYPE_CTRL_FAN_DIVISION));
> +
> + regmap_update_bits(regmap, type_params[type].ctrl_reg,
> + TYPE_CTRL_FAN_MASK, reg_value);
> + regmap_update_bits(regmap, type_params[type].ctrl_reg1,
> + TYPE_CTRL_FAN1_MASK, unit << 16);
> +}
> +
> +static void aspeed_set_fan_tach_ch_enable(struct regmap *regmap, u8 fan_tach_ch,
> + bool enable)
> +{
> + regmap_update_bits(regmap, ASPEED_PTCR_CTRL,
> + ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch),
> + enable ?
> + ASPEED_PTCR_CTRL_FAN_NUM_EN(fan_tach_ch) : 0);
> +}
> +
> +static void aspeed_set_fan_tach_ch_source(struct regmap *regmap, u8 fan_tach_ch,
> + u8 fan_tach_ch_source)
> +{
> + u32 reg_value1 = ((fan_tach_ch_source & 0x3) <<
> + TACH_PWM_SOURCE_BIT01(fan_tach_ch));
> + u32 reg_value2 = (((fan_tach_ch_source & 0x4) >> 2) <<
> + TACH_PWM_SOURCE_BIT2(fan_tach_ch));
> +
> + regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE,
> + TACH_PWM_SOURCE_MASK_BIT01(fan_tach_ch),
> + reg_value1);
> +
> + regmap_update_bits(regmap, ASPEED_PTCR_TACH_SOURCE_EXT,
> + TACH_PWM_SOURCE_MASK_BIT2(fan_tach_ch),
> + reg_value2);
> +}
> +
> +static void aspeed_set_pwm_port_fan_ctrl(struct aspeed_pwm_tacho_data *priv,
> + u8 index, u8 fan_ctrl)
> +{
> + u16 period, dc_time_on;
> +
> + period = priv->type_pwm_clock_unit[priv->pwm_port_type[index]];
> + period += 1;
> + dc_time_on = (fan_ctrl * period) / PWM_MAX;
> +
> + if (dc_time_on == 0) {
> + aspeed_set_pwm_port_enable(priv->regmap, index, false);
> + } else {
> + if (dc_time_on == period)
> + dc_time_on = 0;
> +
> + aspeed_set_pwm_port_duty_rising_falling(priv->regmap, index, 0,
> + dc_time_on);
> + aspeed_set_pwm_port_enable(priv->regmap, index, true);
> + }
> +}
> +
> +static u32 aspeed_get_fan_tach_ch_measure_period(struct aspeed_pwm_tacho_data
> + *priv, u8 type)
> +{
> + u32 clk;
> + u16 tacho_unit;
> + u8 clk_unit, div_h, div_l, tacho_div;
> +
> + clk = priv->clk_freq;
> + clk_unit = priv->type_pwm_clock_unit[type];
> + div_h = priv->type_pwm_clock_division_h[type];
> + div_h = 0x1 << div_h;
> + div_l = priv->type_pwm_clock_division_l[type];
> + if (div_l == 0)
> + div_l = 1;
> + else
> + div_l = div_l * 2;
> +
> + tacho_unit = priv->type_fan_tach_unit[type];
> + tacho_div = priv->type_fan_tach_clock_division[type];
> +
> + tacho_div = 0x4 << (tacho_div * 2);
> + return clk / (clk_unit * div_h * div_l * tacho_div * tacho_unit);
> +}
> +
> +static u32 aspeed_get_fan_tach_ch_rpm(struct aspeed_pwm_tacho_data *priv,
> + u8 fan_tach_ch)
> +{
> + u32 raw_data, tach_div, clk_source, timeout = 0, sec, val;
> + u8 fan_tach_ch_source, type;
> +
> + regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0);
> + regmap_write(priv->regmap, ASPEED_PTCR_TRIGGER, 0x1 << fan_tach_ch);
> +
> + fan_tach_ch_source = priv->fan_tach_ch_source[fan_tach_ch];
> + type = priv->pwm_port_type[fan_tach_ch_source];
> +
> + sec = (1000 / aspeed_get_fan_tach_ch_measure_period(priv, type));
> +
> + msleep(sec);
> +
> + while (!(regmap_read(priv->regmap, ASPEED_PTCR_RESULT, &val))
> + & !(val & RESULT_STATUS_MASK)) {
> + timeout++;
> + if (timeout > 1)
> + return 0;
> + msleep(sec);
> + }
> +
> + raw_data = val & RESULT_VALUE_MASK;
> + tach_div = priv->type_fan_tach_clock_division[type];
> + tach_div = 0x4 << (tach_div * 2);
> + clk_source = priv->clk_freq;
> +
> + if (raw_data == 0)
> + return 0;
> +
> + return (clk_source * 60) / (2 * raw_data * tach_div);
> +}
> +
> +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> + int index = sensor_attr->index;
> + int ret;
> + struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
> + long fan_ctrl;
> +
> + ret = kstrtol(buf, 10, &fan_ctrl);
> + if (ret != 0)
> + return ret;
> +
> + if (fan_ctrl < 0 || fan_ctrl > PWM_MAX)
> + return -EINVAL;
> +
> + if (priv->pwm_port_fan_ctrl[index] == fan_ctrl)
> + return count;
> +
> + priv->pwm_port_fan_ctrl[index] = fan_ctrl;
> + aspeed_set_pwm_port_fan_ctrl(priv, index, fan_ctrl);
> +
> + return count;
> +}
> +
> +static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> + int index = sensor_attr->index;
> + struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", priv->pwm_port_fan_ctrl[index]);
> +}
> +
> +static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> + int index = sensor_attr->index;
> + u32 rpm;
> + struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
> +
> + rpm = aspeed_get_fan_tach_ch_rpm(priv, index);
> +
> + return sprintf(buf, "%u\n", rpm);
> +}
> +
> +static umode_t pwm_is_visible(struct kobject *kobj,
> + struct attribute *a, int index)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
> +
> + if (!priv->pwm_present[index])
> + return 0;
> + return a->mode;
> +}
> +
> +static umode_t fan_dev_is_visible(struct kobject *kobj,
> + struct attribute *a, int index)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct aspeed_pwm_tacho_data *priv = dev_get_drvdata(dev);
> +
> + if (!priv->fan_tach_present[index])
> + return 0;
> + return a->mode;
> +}
> +
> +static SENSOR_DEVICE_ATTR(pwm0, 0644,
> + show_pwm, set_pwm, 0);
> +static SENSOR_DEVICE_ATTR(pwm1, 0644,
> + show_pwm, set_pwm, 1);
> +static SENSOR_DEVICE_ATTR(pwm2, 0644,
> + show_pwm, set_pwm, 2);
> +static SENSOR_DEVICE_ATTR(pwm3, 0644,
> + show_pwm, set_pwm, 3);
> +static SENSOR_DEVICE_ATTR(pwm4, 0644,
> + show_pwm, set_pwm, 4);
> +static SENSOR_DEVICE_ATTR(pwm5, 0644,
> + show_pwm, set_pwm, 5);
> +static SENSOR_DEVICE_ATTR(pwm6, 0644,
> + show_pwm, set_pwm, 6);
> +static SENSOR_DEVICE_ATTR(pwm7, 0644,
> + show_pwm, set_pwm, 7);
> +static struct attribute *pwm_dev_attrs[] = {
> + &sensor_dev_attr_pwm0.dev_attr.attr,
> + &sensor_dev_attr_pwm1.dev_attr.attr,
> + &sensor_dev_attr_pwm2.dev_attr.attr,
> + &sensor_dev_attr_pwm3.dev_attr.attr,
> + &sensor_dev_attr_pwm4.dev_attr.attr,
> + &sensor_dev_attr_pwm5.dev_attr.attr,
> + &sensor_dev_attr_pwm6.dev_attr.attr,
> + &sensor_dev_attr_pwm7.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group pwm_dev_group = {
> + .attrs = pwm_dev_attrs,
> + .is_visible = pwm_is_visible,
> +};
> +
> +static SENSOR_DEVICE_ATTR(fan0_input, 0444,
> + show_rpm, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan1_input, 0444,
> + show_rpm, NULL, 1);
> +static SENSOR_DEVICE_ATTR(fan2_input, 0444,
> + show_rpm, NULL, 2);
> +static SENSOR_DEVICE_ATTR(fan3_input, 0444,
> + show_rpm, NULL, 3);
> +static SENSOR_DEVICE_ATTR(fan4_input, 0444,
> + show_rpm, NULL, 4);
> +static SENSOR_DEVICE_ATTR(fan5_input, 0444,
> + show_rpm, NULL, 5);
> +static SENSOR_DEVICE_ATTR(fan6_input, 0444,
> + show_rpm, NULL, 6);
> +static SENSOR_DEVICE_ATTR(fan7_input, 0444,
> + show_rpm, NULL, 7);
> +static SENSOR_DEVICE_ATTR(fan8_input, 0444,
> + show_rpm, NULL, 8);
> +static SENSOR_DEVICE_ATTR(fan9_input, 0444,
> + show_rpm, NULL, 9);
> +static SENSOR_DEVICE_ATTR(fan10_input, 0444,
> + show_rpm, NULL, 10);
> +static SENSOR_DEVICE_ATTR(fan11_input, 0444,
> + show_rpm, NULL, 11);
> +static SENSOR_DEVICE_ATTR(fan12_input, 0444,
> + show_rpm, NULL, 12);
> +static SENSOR_DEVICE_ATTR(fan13_input, 0444,
> + show_rpm, NULL, 13);
> +static SENSOR_DEVICE_ATTR(fan14_input, 0444,
> + show_rpm, NULL, 14);
> +static SENSOR_DEVICE_ATTR(fan15_input, 0444,
> + show_rpm, NULL, 15);
> +static struct attribute *fan_dev_attrs[] = {
> + &sensor_dev_attr_fan0_input.dev_attr.attr,
> + &sensor_dev_attr_fan1_input.dev_attr.attr,
> + &sensor_dev_attr_fan2_input.dev_attr.attr,
> + &sensor_dev_attr_fan3_input.dev_attr.attr,
> + &sensor_dev_attr_fan4_input.dev_attr.attr,
> + &sensor_dev_attr_fan5_input.dev_attr.attr,
> + &sensor_dev_attr_fan6_input.dev_attr.attr,
> + &sensor_dev_attr_fan7_input.dev_attr.attr,
> + &sensor_dev_attr_fan8_input.dev_attr.attr,
> + &sensor_dev_attr_fan9_input.dev_attr.attr,
> + &sensor_dev_attr_fan10_input.dev_attr.attr,
> + &sensor_dev_attr_fan11_input.dev_attr.attr,
> + &sensor_dev_attr_fan12_input.dev_attr.attr,
> + &sensor_dev_attr_fan13_input.dev_attr.attr,
> + &sensor_dev_attr_fan14_input.dev_attr.attr,
> + &sensor_dev_attr_fan15_input.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group fan_dev_group = {
> + .attrs = fan_dev_attrs,
> + .is_visible = fan_dev_is_visible,
> +};
> +
> +/*
> + * The clock type is type M :
> + * The PWM frequency = 24MHz / (type M clock division L bit *
> + * type M clock division H bit * (type M PWM period bit + 1))
> + */
> +static void aspeed_create_type(struct aspeed_pwm_tacho_data *priv)
> +{
> + priv->type_pwm_clock_division_h[TYPEM] = M_PWM_DIV_H;
> + priv->type_pwm_clock_division_l[TYPEM] = M_PWM_DIV_L;
> + priv->type_pwm_clock_unit[TYPEM] = M_PWM_PERIOD;
> + aspeed_set_pwm_clock_values(priv->regmap, TYPEM, M_PWM_DIV_H,
> + M_PWM_DIV_L, M_PWM_PERIOD);
> + aspeed_set_tacho_type_enable(priv->regmap, TYPEM, true);
> + priv->type_fan_tach_clock_division[TYPEM] = M_TACH_CLK_DIV;
> + priv->type_fan_tach_unit[TYPEM] = M_TACH_UNIT;
> + aspeed_set_tacho_type_values(priv->regmap, TYPEM, M_TACH_MODE,
> + M_TACH_UNIT, M_TACH_CLK_DIV);
> +}
> +
> +static void aspeed_create_pwm_port(struct aspeed_pwm_tacho_data *priv,
> + u8 pwm_port)
> +{
> + aspeed_set_pwm_port_enable(priv->regmap, pwm_port, true);
> + priv->pwm_present[pwm_port] = true;
> +
> + priv->pwm_port_type[pwm_port] = TYPEM;
> + aspeed_set_pwm_port_type(priv->regmap, pwm_port, TYPEM);
> +
> + priv->pwm_port_fan_ctrl[pwm_port] = INIT_FAN_CTRL;
> + aspeed_set_pwm_port_fan_ctrl(priv, pwm_port, INIT_FAN_CTRL);
> +}
> +
> +static void aspeed_create_fan_tach_channel(struct aspeed_pwm_tacho_data *priv,
> + u8 *fan_tach_ch,
> + int count,
> + u8 pwm_source)
> +{
> + u8 val, index;
> +
> + for (val = 0; val < count; val++) {
> + index = fan_tach_ch[val];
> + aspeed_set_fan_tach_ch_enable(priv->regmap, index, true);
> + priv->fan_tach_present[index] = true;
> + priv->fan_tach_ch_source[index] = pwm_source;
> + aspeed_set_fan_tach_ch_source(priv->regmap, index, pwm_source);
> + }
> +}
> +
> +static int aspeed_create_fan(struct device *dev,
> + struct device_node *child,
> + struct aspeed_pwm_tacho_data *priv)
> +{
> + u8 *fan_tach_ch;
> + u32 pwm_port;
> + int ret, count;
> +
> + ret = of_property_read_u32(child, "reg", &pwm_port);
> + if (ret)
> + return ret;
> + aspeed_create_pwm_port(priv, (u8)pwm_port);
> +
> + count = of_property_count_u8_elems(child, "aspeed,fan-tach-ch");
> + if (count < 1)
> + return -EINVAL;
> + fan_tach_ch = devm_kzalloc(dev, sizeof(*fan_tach_ch) * count,
> + GFP_KERNEL);
> + if (!fan_tach_ch)
> + return -ENOMEM;
> + ret = of_property_read_u8_array(child, "aspeed,fan-tach-ch",
> + fan_tach_ch, count);
> + if (ret)
> + return ret;
> + aspeed_create_fan_tach_channel(priv, fan_tach_ch, count, pwm_port);
> +
> + return 0;
> +}
> +
> +static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np, *child;
> + struct aspeed_pwm_tacho_data *priv;
> + void __iomem *regs;
> + struct resource *res;
> + struct device *hwmon;
> + struct clk *clk;
> + int ret;
> +
> + np = dev->of_node;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOENT;
> + regs = devm_ioremap_resource(dev, res);
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> + priv->regmap = devm_regmap_init(dev, NULL, regs,
> + &aspeed_pwm_tacho_regmap_config);
> + if (IS_ERR(priv->regmap))
> + return PTR_ERR(priv->regmap);
> + regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE, 0);
> + regmap_write(priv->regmap, ASPEED_PTCR_TACH_SOURCE_EXT, 0);
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk))
> + return -ENODEV;
> + priv->clk_freq = clk_get_rate(clk);
> + aspeed_set_clock_enable(priv->regmap, true);
> + aspeed_set_clock_source(priv->regmap, 0);
> +
> + aspeed_create_type(priv);
> +
> + for_each_child_of_node(np, child) {
> + ret = aspeed_create_fan(dev, child, priv);
> + of_node_put(child);
> + if (ret)
> + return ret;
> + }
> + of_node_put(np);
> +
> + priv->groups[0] = &pwm_dev_group;
> + priv->groups[1] = &fan_dev_group;
> + priv->groups[2] = NULL;
> + hwmon = devm_hwmon_device_register_with_groups(dev,
> + "aspeed_pwm_tacho",
> + priv, priv->groups);
> +
> + return PTR_ERR_OR_ZERO(hwmon);
> +}
> +
> +static const struct of_device_id of_pwm_tacho_match_table[] = {
> + { .compatible = "aspeed,ast2400-pwm-tacho", },
> + { .compatible = "aspeed,ast2500-pwm-tacho", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_pwm_tacho_match_table);
> +
> +static struct platform_driver aspeed_pwm_tacho_driver = {
> + .probe = aspeed_pwm_tacho_probe,
> + .driver = {
> + .name = "aspeed_pwm_tacho",
> + .of_match_table = of_pwm_tacho_match_table,
> + },
> +};
> +
> +module_platform_driver(aspeed_pwm_tacho_driver);
> +
> +MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
> +MODULE_DESCRIPTION("ASPEED PWM and Fan Tacho device driver");
> +MODULE_LICENSE("GPL");
> --
> 2.12.2.715.g7642488e1d-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH linux v6 1/2] Documentation: dt-bindings: Document bindings for ASPEED AST2400/AST2500 PWM and Fan tach controller device driver
From: Guenter Roeck @ 2017-04-04 12:52 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan,
joel-U3u1mxZcP9KHXe+LvDLADg, jdelvare-IBi9RG/b67k,
linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, corbet-T1hC0tSOHrs,
linux-doc-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170403233025.26952-2-jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 04/03/2017 04:30 PM, Jaghathiswari Rankappagounder Natarajan wrote:
> This binding provides interface for adding values related to ASPEED
> AST2400/2500 PWM and Fan tach controller support.
> The PWM controller can support upto 8 PWM output ports.
> The Fan tach controller can support upto 16 tachometer inputs.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Applied with Acked-by: From Rob (for v5 with the requested changes made).
Thanks,
Guenter
> ---
> v6:
> - Changed to aspeed,fan-tach-ch
>
> v5:
> - Changed the naming scheme to aspeed,ast2400/2500-pwm-tacho
> - Removed gpio pin muxing
> - Added aspeed vendor prefix for fan-tach-ch
> - Changed to fan@0/1
> - Changed reg to 32 bits
>
> v4:
> - Used 'reg'
>
> v3:
> - Made the structure more common
>
> v2:
> - Removed '_' in node and property names
> - Gave some explanation for the properties used
>
> .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> new file mode 100644
> index 000000000000..ac0a69ab0755
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> @@ -0,0 +1,68 @@
> +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver
> +
> +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho
> +controller can support upto 16 Fan tachometer inputs.
> +
> +There can be upto 8 fans supported. Each fan can have one PWM output and
> +one/two Fan tach inputs.
> +
> +Required properties for pwm-tacho node:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 1.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
> +
> +- compatible : should be "aspeed,ast2400-pwm-tacho" for AST2400 and
> + "aspeed,ast2500-pwm-tacho" for AST2500.
> +
> +- clocks : a fixed clock providing input clock frequency(PWM
> + and Fan Tach clock)
> +
> +fan subnode format:
> +===================
> +Under fan subnode there can upto 8 child nodes, with each child node
> +representing a fan. If there are 8 fans each fan can have one PWM port and
> +one/two Fan tach inputs.
> +
> +Required properties for each child node:
> +- reg : should specify PWM source port.
> + integer value in the range 0 to 7 with 0 indicating PWM port A and
> + 7 indicating PWM port H.
> +
> +- aspeed,fan-tach-ch : should specify the Fan tach input channel.
> + integer value in the range 0 through 15, with 0 indicating
> + Fan tach channel 0 and 15 indicating Fan tach channel 15.
> + Atleast one Fan tach input channel is required.
> +
> +Examples:
> +
> +pwm_tacho_fixed_clk: fixedclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24000000>;
> +}
> +
> +pwm_tacho: pwmtachocontroller@1e786000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x1E786000 0x1000>;
> + compatible = "aspeed,ast2500-pwm-tacho";
> + clocks = <&pwm_tacho_fixed_clk>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default>;
> +
> + fan@0 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> + };
> +
> + fan@1 {
> + reg = <0x01>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x01 0x02>;
> + };
> +};
> --
> 2.12.2.715.g7642488e1d-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: arm: amlogic: Add SoC information bindings
From: Neil Armstrong @ 2017-04-04 12:49 UTC (permalink / raw)
To: Rob Herring
Cc: Arnd Bergmann, Kevin Hilman, Carlo Caione,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Linux ARM,
Linux Kernel Mailing List,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKWi=sNgOTTw6+PAxwgFyFQMw6BiUz0uNuG4kWr+ORO_g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04/04/2017 02:26 PM, Rob Herring wrote:
> On Tue, Apr 4, 2017 at 3:51 AM, Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>> On 04/03/2017 06:34 PM, Rob Herring wrote:
>>> On Fri, Mar 31, 2017 at 04:10:30PM +0200, Neil Armstrong wrote:
>>>> On 03/31/2017 03:44 PM, Arnd Bergmann wrote:
>>>>> On Fri, Mar 31, 2017 at 10:47 AM, Neil Armstrong
>>>>> <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>>>>>> Add bindings for the SoC information register of the Amlogic SoCs.
>>>>>>
>>>>>> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>>>> ---
>>>>>> Documentation/devicetree/bindings/arm/amlogic.txt | 20 ++++++++++++++++++++
>>>>>> 1 file changed, 20 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/arm/amlogic.txt b/Documentation/devicetree/bindings/arm/amlogic.txt
>>>>>> index bfd5b55..b850985 100644
>>>>>> --- a/Documentation/devicetree/bindings/arm/amlogic.txt
>>>>>> +++ b/Documentation/devicetree/bindings/arm/amlogic.txt
>>>>>> @@ -52,3 +52,23 @@ Board compatible values:
>>>>>> - "amlogic,q201" (Meson gxm s912)
>>>>>> - "nexbox,a95x" (Meson gxbb or Meson gxl s905x)
>>>>>> - "nexbox,a1" (Meson gxm s912)
>>>>>> +
>>>>>> +Amlogic Meson GX SoCs Information
>>>>>> +----------------------------------
>>>>>> +
>>>>>> +The Meson SoCs have a Product Register that allows to retrieve SoC type,
>>>>>> +package and revision information. If present, a device node for this register
>>>>>> +should be added.
>>>>>> +
>>>>>> +Required properties:
>>>>>> + - compatible: For Meson GX SoCs, must be "amlogic,meson-gx-socinfo".
>>>>>> + - reg: Base address and length of the register block.
>>>>>> +
>>>>>> +Examples
>>>>>> +--------
>>>>>> +
>>>>>> + chipid@220 {
>>>>>> + compatible = "amlogic,meson-gx-socinfo";
>>>>>> + reg = <0x0 0x00220 0x0 0x4>;
>>>>>> + };
>>>>>> +
>>>>>
>>>>> The register location would hint that this is in the middle of some block of
>>>>> random registers, i.e. a syscon or some unrelated device.
>>>>>
>>>>> Are you sure that "socinfo" is the actual name of the IP block and that
>>>>> it only has a single 32-bit register?
>>>>>
>>>>> Arnd
>>>>>
>>>>
>>>> Hi Arnd,
>>>>
>>>> I'm sorry I did not find any relevant registers in the docs or source code describing
>>>> it in a specific block of registers, and no close enough register definitions either.
>>>> They may be used by the secure firmware I imagine.
>>>>
>>>> For the register name, Amlogic refers it to "cpu_version" in their code, but it really
>>>> gives some details on the whole SoC and package, and socinfo seems better.
>>>
>>> A register at address 0x220 seems a bit strange (unless there's ranges
>>> you're not showing), but ROM code at this address would be fairly
>>> typical. And putting version information into the ROM is also common.
>>>
>>> Rob
>>>
>>
>> Hi Rob.
>>
>> Indeed it's part of a larger range :
>> aobus: aobus@c8100000 {
>> compatible = "simple-bus";
>> reg = <0x0 0xc8100000 0x0 0x100000>;
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges = <0x0 0x0 0x0 0xc8100000 0x0 0x100000>;
>>
>>
>> While scrubbing on the uboot source, I found a sort of block of registers dedicated to communicate with
>> the secure firmware :
>> AO_SEC_REG0 0x140
>> AO_SEC_REG1 0x144
>> AO_SEC_REG2 0x148
>> AO_SEC_TMODE_PWD0 0x160
>> AO_SEC_TMODE_PWD1 0x164
>> AO_SEC_TMODE_PWD2 0x168
>> AO_SEC_TMODE_PWD3 0x16C
>> AO_SEC_SCRATCH 0x17C
>> AO_SEC_JTAG_PWD0 0x180
>> AO_SEC_JTAG_PWD1 0x184
>> AO_SEC_JTAG_PWD2 0x188
>> AO_SEC_JTAG_PWD3 0x18C
>> AO_SEC_JTAG_SEC_CNTL 0x190
>> AO_SEC_JTAG_PWD_ADDR0 0x194
>> AO_SEC_JTAG_PWD_ADDR1 0x198
>> AO_SEC_JTAG_PWD_ADDR2 0x19C
>> AO_SEC_JTAG_PWD_ADDR3 0x1A0
>> AO_SEC_SHARED_AHB_SRAM_REG0_0 0x1C0
>> AO_SEC_SHARED_AHB_SRAM_REG0_1 0x1C4
>> AO_SEC_SHARED_AHB_SRAM_REG0_2 0x1C8
>> AO_SEC_SHARED_AHB_SRAM_REG1_0 0x1CC
>> AO_SEC_SHARED_AHB_SRAM_REG1_1 0x1D0
>> AO_SEC_SHARED_AHB_SRAM_REG1_2 0x1D4
>> AO_SEC_SHARED_AHB_SRAM_REG2_0 0x1D8
>> AO_SEC_SHARED_AHB_SRAM_REG2_1 0x1DC
>> AO_SEC_SHARED_AHB_SRAM_REG2_2 0x1E0
>> AO_SEC_SHARED_AHB_SRAM_REG3_0 0x1E4
>> AO_SEC_SHARED_AHB_SRAM_REG3_1 0x1E8
>> AO_SEC_SHARED_AHB_SRAM_REG3_2 0x1EC
>> AO_SEC_AO_AHB_SRAM_REG0_0 0x1F0
>> AO_SEC_AO_AHB_SRAM_REG0_1 0x1F4
>> AO_SEC_AO_AHB_SRAM_REG1_0 0x1F8
>> AO_SEC_AO_AHB_SRAM_REG1_1 0x1FC
>> AO_SEC_SD_CFG8 0x220
>> AO_SEC_SD_CFG9 0x224
>> AO_SEC_SD_CFG10 0x228
>> AO_SEC_SD_CFG11 0x22C
>> AO_SEC_SD_CFG12 0x230
>> AO_SEC_SD_CFG13 0x234
>> AO_SEC_SD_CFG14 0x238
>> AO_SEC_SD_CFG15 0x23C
>> AO_SEC_GP_CFG0 0x240
>> AO_SEC_GP_CFG1 0x244
>> AO_SEC_GP_CFG2 0x248
>> AO_SEC_GP_CFG3 0x24C
>> AO_SEC_GP_CFG4 0x250
>> AO_SEC_GP_CFG5 0x254
>> AO_SEC_GP_CFG6 0x258
>> AO_SEC_GP_CFG7 0x25C
>> AO_SEC_GP_CFG8 0x260
>> AO_SEC_GP_CFG9 0x264
>> AO_SEC_GP_CFG10 0x268
>> AO_SEC_GP_CFG11 0x26C
>> AO_SEC_GP_CFG12 0x270
>> AO_SEC_GP_CFG13 0x274
>> AO_SEC_GP_CFG14 0x278
>> AO_SEC_GP_CFG15 0x27C
>>
>>
>> As you see, the register we use here is AO_SEC_SD_CFG8...
>>
>> Should I define all this block as simple-mfd and refer to it as a regmap ?
>>
>> aobus: aobus@c8100000 {
>> compatible = "simple-bus";
>> reg = <0x0 0xc8100000 0x0 0x100000>;
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges = <0x0 0x0 0x0 0xc8100000 0x0 0x100000>;
>>
>> ao_secure: ao-secure@140 {
>> compatible = "amlogic,meson-gx-ao-secure", "simple-mfd";
>> reg = <0x0 0x140 0x0 0x140>;
>> };
>> };
>>
>> chipid {
>> compatible = "amlogic,meson-gx-socinfo";
>> ao-secure = <&ao_secure>;
>> chip-info-reg = <0xe0>;
>
> Why even divide it up further in DT? IMO, describing single
> registers/address in DT is too fine grained.
>
> Rob
>
Rob, I don't get it.
Maybe something like that ?
aobus: aobus@c8100000 {
compatible = "simple-bus";
reg = <0x0 0xc8100000 0x0 0x100000>;
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xc8100000 0x0 0x100000>;
ao_secure: ao-secure@140 {
compatible = "amlogic,meson-gx-ao-secure", "simple-mfd", "simple-bus";
reg = <0x0 0x140 0x0 0x140>;
#address-cells = <1>;
#size-cells = <1>;
chipid@e0 {
compatible = "amlogic,meson-gx-socinfo";
reg = <0xe0 0x4>;
};
};
};
Concerning the fine graining, I'm sorry but the actual information comes from a single register here...
Neil
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V10 00/12] IOMMU probe deferral support
From: Robin Murphy @ 2017-04-04 12:49 UTC (permalink / raw)
To: Sricharan R, will.deacon, joro, lorenzo.pieralisi, iommu,
linux-arm-kernel, linux-arm-msm, m.szyprowski, bhelgaas,
linux-pci, linux-acpi, tn, hanjun.guo, okaya, robh+dt,
frowand.list, devicetree, linux-kernel, sudeep.holla, rjw, lenb,
catalin.marinas, arnd, linux-arch, gregkh
In-Reply-To: <1491301105-5274-1-git-send-email-sricharan@codeaurora.org>
On 04/04/17 11:18, Sricharan R wrote:
> This series calls the dma ops configuration for the devices
> at a generic place so that it works for all busses.
> The dma_configure_ops for a device is now called during
> the device_attach callback just before the probe of the
> bus/driver is called. Similarly dma_deconfigure is called during
> device/driver_detach path.
>
> pci_bus_add_devices (platform/amba)(_device_create/driver_register)
> | |
> pci_bus_add_device (device_add/driver_register)
> | |
> device_attach device_initial_probe
> | |
> __device_attach_driver __device_attach_driver
> |
> driver_probe_device
> |
> really_probe
> |
> dma_configure
>
> Similarly on the device/driver_unregister path __device_release_driver is
> called which inturn calls dma_deconfigure.
>
> Rebased the series against mainline 4.11-rc5. Applies and builds cleanly
> against mainline and linux-next, iommu-next.
>
> * Tested with platform and pci devices for probe deferral
> and reprobe on arm64 based platform.
Sricharan, thanks for keeping this going - I really think we're there now :)
Joerg, I realise that at -rc5 time is getting on a bit already, but even
the non-vintage parts of the series are pretty mature now so it would be
nice to at least give it a spin in -next. If you don't quite share my
confidence for landing it in 4.12, please consider it for early next
cycle to get a full workout.
Thanks,
Robin.
> Previous post of this series [8].
>
> Please note that, i have kept the tested/acked tags intact from V8
> because V9/10 were for more fixes that was added, so the original
> tags that was given for the functional testing remains the same.
>
> [V10]
> * Rebased on top of 4.11-rc5.
>
> * Fixed coherent_dma_mask 64bit overflow issue [8]
> for OF. The fix for OF was added as a separate
> patch#6, since the issue is true even without probe deferral,
> but gets reproduced with the probe deferral series.
> Added Lorenzo's ACPI fix for coherent_dma_mask overflow
> and the fix for dma_configure getting called more than
> once for the same device.
>
> * Also fixed an build issue caught by kbuild robot for
> m68k arch. The issue was dma_(de)configure was not
> getting defined for !CONFIG_HAS_DMA, so fixed that as well.
>
> [V9]
> * Rebased on top of 4.11-rc1.
>
> * Merged Robin's fixes for legacy binding issue,
> pci devices with no iommu-map property and deferencing
> of_iommu_table after init.
>
> [V8]
> * Picked up all the acks and tested tags from Marek and
> Hanjun for DT and ACPI patches respectively, since
> no functional changes was done.
>
> * Addressed Minor comments Sinan and Bjorn.
>
> * Added Robin's fix for fixing the deferencing NULL for
> of_iommu_table after init in patch #2.
>
> * Rebased it on top of linux-next
>
> [V7]
> * Updated the subject and commit log for patch #6 as per
> comments from Lorenzo. No functional changes.
>
> [V6]
> * Fixed a bug in dma_configure function pointed out by
> Robin.
> * Reordered the patches as per comments from Robin and
> Lorenzo.
> * Added Tags.
>
> [V5]
> * Reworked the pci configuration code hanging outside and
> pushed it to dma_configure as in PATCH#5,6,7.
> Also added a couple of patches that Lorenzo provided for
> correcting the Probe deferring mechanism in case of
> ACPI devices from here [5].
>
> [V4]
> * Took the reworked patches [2] from Robin's branch and
> rebased on top of Lorenzo's ACPI IORT ARM support series [3].
>
> * Added the patches for moving the dma ops configuration of
> acpi based devices to probe time as well.
> [V3]
> * Removed the patch to split dma_masks/dma_ops configuration
> separately based on review comments that both masks and ops are
> required only during the device probe time.
>
> * Reworked the series based on Generic DT bindings series.
>
> * Added call to iommu's remove_device in the cleanup path for arm and
> arm64.
>
> * Removed the notifier trick in arm64 to handle early device
> registration.
>
> * Added reset of dma_ops in cleanup path for arm based on comments.
>
> * Fixed the pci_iommu_configure path and tested with PCI device as
> well.
>
> * Fixed a bug to return the correct iommu_ops from patch 7 [4] in
> last post.
>
> * Fixed few other cosmetic comments.
>
> [V2]
> * Updated the Initial post to call dma_configure/deconfigure from
> generic code
>
> * Added iommu add_device callback from of_iommu_configure path
>
> [V1]
> * Initial post from Laurent Pinchart [1]
>
> [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
> [2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
> [3] https://lkml.org/lkml/2016/11/21/141
> [4] https://www.mail-archive.com/iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx/msg13940.html
> [5] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral
> [6] http://www.spinics.net/lists/linux-pci/msg57992.html
> [7] https://www.spinics.net/lists/arm-kernel/msg556209.html
> [8] http://patchwork.ozlabs.org/patch/743898/
>
> Laurent Pinchart (3):
> of: dma: Move range size workaround to of_dma_get_range()
> of: dma: Make of_dma_deconfigure() public
> iommu: of: Handle IOMMU lookup failure with deferred probing or error
>
> Lorenzo Pieralisi (2):
> ACPI/IORT: Add function to check SMMUs drivers presence
> ACPI/IORT: Remove linker section for IORT entries probing
>
> Robin Murphy (3):
> iommu/of: Refactor of_iommu_configure() for error handling
> iommu/of: Prepare for deferred IOMMU configuration
> iommu/arm-smmu: Clean up early-probing workarounds
>
> Sricharan R (4):
> of: device: Fix overflow of coherent_dma_mask
> of/acpi: Configure dma operations at probe time for platform/amba/pci
> bus devices
> drivers: acpi: Handle IOMMU lookup failure with deferred probing or
> error
> arm64: dma-mapping: Remove the notifier trick to handle early setting
> of dma_ops
>
> arch/arm64/mm/dma-mapping.c | 142 +++++---------------------------------
> drivers/acpi/arm64/iort.c | 48 ++++++++++++-
> drivers/acpi/glue.c | 5 --
> drivers/acpi/scan.c | 11 ++-
> drivers/base/dd.c | 9 +++
> drivers/base/dma-mapping.c | 41 +++++++++++
> drivers/iommu/arm-smmu-v3.c | 46 +-----------
> drivers/iommu/arm-smmu.c | 110 +++++++++++++----------------
> drivers/iommu/of_iommu.c | 126 ++++++++++++++++++++++++---------
> drivers/of/address.c | 20 +++++-
> drivers/of/device.c | 36 +++++-----
> drivers/of/platform.c | 10 +--
> drivers/pci/probe.c | 28 --------
> include/acpi/acpi_bus.h | 2 +-
> include/asm-generic/vmlinux.lds.h | 1 -
> include/linux/acpi.h | 7 +-
> include/linux/acpi_iort.h | 3 -
> include/linux/dma-mapping.h | 12 ++++
> include/linux/of_device.h | 10 ++-
> 19 files changed, 329 insertions(+), 338 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
From: Sakari Ailus @ 2017-04-04 12:47 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
fabio.estevam-3arQi8VN3Tc, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
mchehab-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
nick-gcszYUEDH4VrovVCs/uTlw, markus.heiser-O6JHGLzbNUwb1SvskN2V4Q,
p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g,
arnd-r2nGTMty4D4, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
jean-christophe.trotin-qxv4g6HH51o,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
shuah-DgEjT+Ai2ygdnm+yROfE0A, sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-me
In-Reply-To: <1490661656-10318-18-git-send-email-steve_longerbeam-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Hi Steve, Philipp and Pavel,
On Mon, Mar 27, 2017 at 05:40:34PM -0700, Steve Longerbeam wrote:
> From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> This driver can handle SoC internal and external video bus multiplexers,
> controlled either by register bit fields or by a GPIO. The subdevice
> passes through frame interval and mbus configuration of the active input
> to the output side.
The MUX framework is already in linux-next. Could you use that instead of
adding new driver + bindings that are not compliant with the MUX framework?
I don't think it'd be much of a change in terms of code, using the MUX
framework appears quite simple.
In general the driver looks pretty good, especially regarding the user space
API implementation which is important for use with other drivers.
I have some more detailed comments below.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> - fixed a cut&paste error in vidsw_remove(): v4l2_async_register_subdev()
> should be unregister.
>
> - added media_entity_cleanup() to vidsw_remove().
>
> - added missing MODULE_DEVICE_TABLE().
> Suggested-by: Javier Martinez Canillas <javier-0uQlZySMnqxg9hUCZPvPmw@public.gmane.org>
>
> - there was a line left over from a previous iteration that negated
> the new way of determining the pad count just before it which
> has been removed (num_pads = of_get_child_count(np)).
>
> - removed [gs]_frame_interval ops. timeperframe is not used anywhwere
> in this subdev, and currently it has no control over frame rate.
>
> - add link_validate to media_entity_operations.
>
> - moved devicetree binding doc to a separate commit.
>
> - Philipp Zabel has developed a set of patches that allow adding
> to the subdev async notifier waiting list using a chaining method
> from the async registered callbacks (v4l2_of_subdev_registered()
> and the prep patches for that). For now, I've removed the use of
> v4l2_of_subdev_registered() for the vidmux driver's registered
> callback. This doesn't affect the functionality of this driver,
> but allows for it to be merged now, before adding the chaining
> support.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/media/platform/Kconfig | 8 +
> drivers/media/platform/Makefile | 2 +
> drivers/media/platform/video-multiplexer.c | 451 +++++++++++++++++++++++++++++
> 3 files changed, 461 insertions(+)
> create mode 100644 drivers/media/platform/video-multiplexer.c
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index ab0bb48..c9b8d9c 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -74,6 +74,14 @@ config VIDEO_M32R_AR_M64278
> To compile this driver as a module, choose M here: the
> module will be called arv.
>
> +config VIDEO_MULTIPLEXER
> + tristate "Video Multiplexer"
> + depends on VIDEO_V4L2_SUBDEV_API && MEDIA_CONTROLLER
> + help
> + This driver provides support for SoC internal N:1 video bus
> + multiplexers controlled by register bitfields as well as external
> + 2:1 video multiplexers controlled by a single GPIO.
> +
> config VIDEO_OMAP3
> tristate "OMAP 3 Camera support"
> depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 8959f6e..d418add 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -27,6 +27,8 @@ obj-$(CONFIG_VIDEO_SH_VEU) += sh_veu.o
>
> obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE) += m2m-deinterlace.o
>
> +obj-$(CONFIG_VIDEO_MULTIPLEXER) += video-multiplexer.o
> +
> obj-$(CONFIG_VIDEO_S3C_CAMIF) += s3c-camif/
> obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/
> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
> diff --git a/drivers/media/platform/video-multiplexer.c b/drivers/media/platform/video-multiplexer.c
> new file mode 100644
> index 0000000..b18c317
> --- /dev/null
> +++ b/drivers/media/platform/video-multiplexer.c
> @@ -0,0 +1,451 @@
> +/*
> + * video stream multiplexer controlled via gpio or syscon
> + *
> + * Copyright (C) 2013 Pengutronix, Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + * Copyright (C) 2016 Pengutronix, Philipp Zabel <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +
> +struct vidsw {
> + struct v4l2_subdev subdev;
> + unsigned int num_pads;
You could use subdev.entity.num_pads instead of caching the value locally.
> + struct media_pad *pads;
> + struct v4l2_mbus_framefmt *format_mbus;
> + struct v4l2_of_endpoint *endpoint;
> + struct regmap_field *field;
> + struct gpio_desc *gpio;
> + int active;
> +};
> +
> +static inline struct vidsw *v4l2_subdev_to_vidsw(struct v4l2_subdev *sd)
> +{
> + return container_of(sd, struct vidsw, subdev);
> +}
> +
> +static void vidsw_set_active(struct vidsw *vidsw, int active)
> +{
> + vidsw->active = active;
> + if (active < 0)
> + return;
> +
> + dev_dbg(vidsw->subdev.dev, "setting %d active\n", active);
> +
> + if (vidsw->field)
> + regmap_field_write(vidsw->field, active);
> + else if (vidsw->gpio)
> + gpiod_set_value(vidsw->gpio, active);
> +}
> +
> +static int vidsw_link_setup(struct media_entity *entity,
> + const struct media_pad *local,
> + const struct media_pad *remote, u32 flags)
> +{
> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> +
> + /* We have no limitations on enabling or disabling our output link */
> + if (local->index == vidsw->num_pads - 1)
> + return 0;
> +
> + dev_dbg(sd->dev, "link setup %s -> %s", remote->entity->name,
> + local->entity->name);
> +
> + if (!(flags & MEDIA_LNK_FL_ENABLED)) {
> + if (local->index == vidsw->active) {
> + dev_dbg(sd->dev, "going inactive\n");
> + vidsw->active = -1;
> + }
> + return 0;
> + }
> +
> + if (vidsw->active >= 0) {
> + struct media_pad *pad;
> +
> + if (vidsw->active == local->index)
> + return 0;
> +
> + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
> + if (pad) {
> + struct media_link *link;
> + int ret;
> +
> + link = media_entity_find_link(pad,
> + &vidsw->pads[vidsw->active]);
> + if (link) {
> + ret = __media_entity_setup_link(link, 0);
I wouldn't implicitly disable a link, even if only one can be active at a
given time. No other drivers do that either.
Perhaps returning an error might be a better thing to do: if you're
reconfiguring the pipeline anyway, there are likely issues elsewhere in it.
We could also change the behaviour later to allow implicit changes but we
can't later on go the other way without breaking the user space.
> + if (ret)
> + return ret;
> + }
> + }
> + }
> +
> + vidsw_set_active(vidsw, local->index);
> +
> + return 0;
> +}
> +
> +static struct media_entity_operations vidsw_ops = {
> + .link_setup = vidsw_link_setup,
> + .link_validate = v4l2_subdev_link_validate,
> +};
> +
> +static bool vidsw_endpoint_disabled(struct device_node *ep)
> +{
> + struct device_node *rpp;
> +
> + if (!of_device_is_available(ep))
ep here is the endpoint, whereas the argument to of_device_is_available()
should correspond to the actual device.
> + return true;
> +
> + rpp = of_graph_get_remote_port_parent(ep);
> + if (!rpp)
> + return true;
> +
> + return !of_device_is_available(rpp);
> +}
> +
> +static int vidsw_async_init(struct vidsw *vidsw, struct device_node *node)
I think I'd arrange this closer to probe as it's related to probe directly.
Up to you.
> +{
> + struct device_node *ep;
> + u32 portno;
> + int numports;
> + int ret;
> + int i;
> + bool active_link = false;
> +
> + numports = vidsw->num_pads;
> +
> + for (i = 0; i < numports - 1; i++)
> + vidsw->pads[i].flags = MEDIA_PAD_FL_SINK;
> + vidsw->pads[numports - 1].flags = MEDIA_PAD_FL_SOURCE;
> +
> + vidsw->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
> + ret = media_entity_pads_init(&vidsw->subdev.entity, numports,
> + vidsw->pads);
> + if (ret < 0)
> + return ret;
> +
> + vidsw->subdev.entity.ops = &vidsw_ops;
> +
> + for_each_endpoint_of_node(node, ep) {
> + struct v4l2_of_endpoint endpoint;
> +
> + v4l2_of_parse_endpoint(ep, &endpoint);
> +
> + portno = endpoint.base.port;
> + if (portno >= numports - 1)
> + continue;
> +
> + if (vidsw_endpoint_disabled(ep)) {
> + dev_dbg(vidsw->subdev.dev,
> + "port %d disabled\n", portno);
> + continue;
> + }
> +
> + vidsw->endpoint[portno] = endpoint;
> +
> + if (portno == vidsw->active)
> + active_link = true;
> + }
> +
> + for (portno = 0; portno < numports - 1; portno++) {
> + if (!vidsw->endpoint[portno].base.local_node)
> + continue;
> +
> + /* If the active input is not connected, use another */
> + if (!active_link) {
> + vidsw_set_active(vidsw, portno);
> + active_link = true;
> + }
> + }
> +
> + return v4l2_async_register_subdev(&vidsw->subdev);
> +}
> +
> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config *cfg)
We should get rid of g_mbus_config() in the long run, but as we don't have
the alternative (frame descriptors) isn't up to the job yet I guess it's ok.
I don't think we'll have too many users for the video switch right now.
> +{
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> + struct media_pad *pad;
> + int ret;
> +
> + if (vidsw->active == -1) {
> + dev_err(sd->dev, "no configuration for inactive mux\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Retrieve media bus configuration from the entity connected to the
> + * active input
> + */
> + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
> + if (pad) {
> + sd = media_entity_to_v4l2_subdev(pad->entity);
> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
> + if (ret == -ENOIOCTLCMD)
> + pad = NULL;
> + else if (ret < 0) {
> + dev_err(sd->dev, "failed to get source configuration\n");
> + return ret;
> + }
> + }
> + if (!pad) {
> + /* Mirror the input side on the output side */
> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
> + if (cfg->type == V4L2_MBUS_PARALLEL ||
> + cfg->type == V4L2_MBUS_BT656)
> + cfg->flags = vidsw->endpoint[vidsw->active].bus.parallel.flags;
> + }
> +
> + return 0;
> +}
> +
> +static int vidsw_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> + struct v4l2_subdev *upstream_sd;
> + struct media_pad *pad;
> +
> + if (vidsw->active == -1) {
> + dev_err(sd->dev, "Can not start streaming on inactive mux\n");
> + return -EINVAL;
> + }
> +
> + pad = media_entity_remote_pad(&sd->entity.pads[vidsw->active]);
> + if (!pad) {
> + dev_err(sd->dev, "Failed to find remote source pad\n");
> + return -ENOLINK;
> + }
> +
> + if (!is_media_entity_v4l2_subdev(pad->entity)) {
> + dev_err(sd->dev, "Upstream entity is not a v4l2 subdev\n");
> + return -ENODEV;
> + }
> +
> + upstream_sd = media_entity_to_v4l2_subdev(pad->entity);
> +
> + return v4l2_subdev_call(upstream_sd, video, s_stream, enable);
Now that we'll have more than two drivers involved in the same pipeline it
becomes necessary to define the behaviour of s_stream() throughout the
pipeline --- i.e. whose responsibility is it to call s_stream() on the
sub-devices in the pipeline?
I can submit a patch for that. I think the way you do it here is good, as it
enables the caller to choose the appropriate behaviour, i.e. start the local
device before or after the upstream sub-device.
> +}
> +
> +static const struct v4l2_subdev_video_ops vidsw_subdev_video_ops = {
> + .g_mbus_config = vidsw_g_mbus_config,
> + .s_stream = vidsw_s_stream,
> +};
> +
> +static struct v4l2_mbus_framefmt *
> +__vidsw_get_pad_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + unsigned int pad, u32 which)
> +{
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> +
> + switch (which) {
> + case V4L2_SUBDEV_FORMAT_TRY:
> + return v4l2_subdev_get_try_format(sd, cfg, pad);
> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> + return &vidsw->format_mbus[pad];
> + default:
> + return NULL;
> + }
> +}
> +
> +static int vidsw_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *sdformat)
> +{
> + sdformat->format = *__vidsw_get_pad_format(sd, cfg, sdformat->pad,
> + sdformat->which);
> + return 0;
> +}
> +
> +static int vidsw_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *sdformat)
> +{
> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
> + struct v4l2_mbus_framefmt *mbusformat;
> +
> + if (sdformat->pad >= vidsw->num_pads)
> + return -EINVAL;
This check is already performed in v4l2-subdev.c.
> +
> + mbusformat = __vidsw_get_pad_format(sd, cfg, sdformat->pad,
> + sdformat->which);
> + if (!mbusformat)
> + return -EINVAL;
> +
> + /* Output pad mirrors active input pad, no limitations on input pads */
Source and sink pads.
> + if (sdformat->pad == (vidsw->num_pads - 1) && vidsw->active >= 0)
I think it'd be cleaner to test for the pad flag instead of the number. Or,
add a macro to obtain the source pad number.
> + sdformat->format = vidsw->format_mbus[vidsw->active];
> +
> + *mbusformat = sdformat->format;
> +
> + return 0;
> +}
> +
> +static struct v4l2_subdev_pad_ops vidsw_pad_ops = {
> + .get_fmt = vidsw_get_format,
> + .set_fmt = vidsw_set_format,
> +};
> +
> +static struct v4l2_subdev_ops vidsw_subdev_ops = {
> + .pad = &vidsw_pad_ops,
> + .video = &vidsw_subdev_video_ops,
> +};
> +
> +static int of_get_reg_field(struct device_node *node, struct reg_field *field)
> +{
> + u32 bit_mask;
> + int ret;
> +
> + ret = of_property_read_u32(node, "reg", &field->reg);
> + if (ret < 0)
> + return ret;
> +
> + ret = of_property_read_u32(node, "bit-mask", &bit_mask);
> + if (ret < 0)
> + return ret;
> +
> + ret = of_property_read_u32(node, "bit-shift", &field->lsb);
> + if (ret < 0)
> + return ret;
I think the above would look nice in a MUX driver. :-)
> +
> + field->msb = field->lsb + fls(bit_mask) - 1;
> +
> + return 0;
> +}
> +
> +static int vidsw_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct of_endpoint endpoint;
> + struct device_node *ep;
> + struct reg_field field;
> + struct vidsw *vidsw;
> + struct regmap *map;
> + unsigned int num_pads;
> + int ret;
> +
> + vidsw = devm_kzalloc(&pdev->dev, sizeof(*vidsw), GFP_KERNEL);
> + if (!vidsw)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, vidsw);
> +
> + v4l2_subdev_init(&vidsw->subdev, &vidsw_subdev_ops);
> + snprintf(vidsw->subdev.name, sizeof(vidsw->subdev.name), "%s",
> + np->name);
> + vidsw->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + vidsw->subdev.dev = &pdev->dev;
> +
> + /*
> + * The largest numbered port is the output port. It determines
> + * total number of pads
> + */
> + num_pads = 0;
You can initialise num_pads in variable declaration.
> + for_each_endpoint_of_node(np, ep) {
> + of_graph_parse_endpoint(ep, &endpoint);
> + num_pads = max(num_pads, endpoint.port + 1);
Port numbers come directly from DT.
Shouldn't num_pads be only the number of pads that have links with actual
physical connections? I.e. if a device is disabled, it shouldn't be
counted here.
> + }
> +
> + if (num_pads < 2) {
> + dev_err(&pdev->dev, "Not enough ports %d\n", num_pads);
> + return -EINVAL;
> + }
> +
> + ret = of_get_reg_field(np, &field);
> + if (ret == 0) {
> + map = syscon_node_to_regmap(np->parent);
> + if (!map) {
> + dev_err(&pdev->dev, "Failed to get syscon register map\n");
> + return PTR_ERR(map);
> + }
> +
> + vidsw->field = devm_regmap_field_alloc(&pdev->dev, map, field);
> + if (IS_ERR(vidsw->field)) {
> + dev_err(&pdev->dev, "Failed to allocate regmap field\n");
> + return PTR_ERR(vidsw->field);
> + }
> +
> + regmap_field_read(vidsw->field, &vidsw->active);
> + } else {
> + if (num_pads > 3) {
> + dev_err(&pdev->dev, "Too many ports %d\n", num_pads);
> + return -EINVAL;
> + }
> +
> + vidsw->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW);
> + if (IS_ERR(vidsw->gpio)) {
> + dev_warn(&pdev->dev,
> + "could not request control gpio: %d\n", ret);
> + vidsw->gpio = NULL;
> + }
> +
> + vidsw->active = gpiod_get_value(vidsw->gpio) ? 1 : 0;
> + }
> +
> + vidsw->num_pads = num_pads;
> + vidsw->pads = devm_kzalloc(&pdev->dev, sizeof(*vidsw->pads) * num_pads,
> + GFP_KERNEL);
> + vidsw->format_mbus = devm_kzalloc(&pdev->dev,
> + sizeof(*vidsw->format_mbus) * num_pads, GFP_KERNEL);
> + vidsw->endpoint = devm_kzalloc(&pdev->dev,
> + sizeof(*vidsw->endpoint) * (num_pads - 1), GFP_KERNEL);
> +
> + ret = vidsw_async_init(vidsw, np);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int vidsw_remove(struct platform_device *pdev)
> +{
> + struct vidsw *vidsw = platform_get_drvdata(pdev);
> + struct v4l2_subdev *sd = &vidsw->subdev;
> +
> + v4l2_async_unregister_subdev(sd);
> + media_entity_cleanup(&sd->entity);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id vidsw_dt_ids[] = {
> + { .compatible = "video-multiplexer", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, vidsw_dt_ids);
> +
> +static struct platform_driver vidsw_driver = {
> + .probe = vidsw_probe,
> + .remove = vidsw_remove,
> + .driver = {
> + .of_match_table = vidsw_dt_ids,
> + .name = "video-multiplexer",
> + },
> +};
> +
> +module_platform_driver(vidsw_driver);
> +
> +MODULE_DESCRIPTION("video stream multiplexer");
> +MODULE_AUTHOR("Sascha Hauer, Pengutronix");
> +MODULE_AUTHOR("Philipp Zabel, Pengutronix");
> +MODULE_LICENSE("GPL");
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V10 07/12] of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices
From: Sricharan R @ 2017-04-04 12:30 UTC (permalink / raw)
To: Robin Murphy, will.deacon, joro, lorenzo.pieralisi, iommu,
linux-arm-kernel, linux-arm-msm, m.szyprowski, bhelgaas,
linux-pci, linux-acpi, tn, hanjun.guo, okaya, robh+dt,
frowand.list, devicetree, linux-kernel, sudeep.holla, rjw, lenb,
catalin.marinas, arnd, linux-arch, gregkh
In-Reply-To: <1d036139-b885-bac6-01c0-78d3ac690d80@arm.com>
Hi Robin,
On 4/4/2017 5:47 PM, Robin Murphy wrote:
> On 04/04/17 11:18, Sricharan R wrote:
>> Configuring DMA ops at probe time will allow deferring device probe when
>> the IOMMU isn't available yet. The dma_configure for the device is
>> now called from the generic device_attach callback just before the
>> bus/driver probe is called. This way, configuring the DMA ops for the
>> device would be called at the same place for all bus_types, hence the
>> deferred probing mechanism should work for all buses as well.
>>
>> pci_bus_add_devices (platform/amba)(_device_create/driver_register)
>> | |
>> pci_bus_add_device (device_add/driver_register)
>> | |
>> device_attach device_initial_probe
>> | |
>> __device_attach_driver __device_attach_driver
>> |
>> driver_probe_device
>> |
>> really_probe
>> |
>> dma_configure
>>
>> Similarly on the device/driver_unregister path __device_release_driver is
>> called which inturn calls dma_deconfigure.
>>
>> This patch changes the dma ops configuration to probe time for
>> both OF and ACPI based platform/amba/pci bus devices.
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>
> It's possible we could subsume {of,acpi}_dma_deconfigure() into
> dma_deconfigure() entirely in future if it becomes clear that neither of
> them will ever need to do anything firmware-specific, but I think for
> now it's probably safer to keep the current symmetry - calling
> arch_teardown_dma_ops() twice is benign (and even if it weren't, I'd say
> it really should be!)
>
Ya, calling it twice should cause no harm. If nothing firmware specific
gets added this could simply be put in deconfigure itself in future.
Thanks for all the reviews !!
Regards,
Sricharan
> Robin.
>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> (drivers/pci part)
>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>>
>> [V10] Added dummy dma_(de)configure functions in case
>> of !CONFIG_HAS_DMA to avoid build breaks.
>>
>> drivers/acpi/glue.c | 5 -----
>> drivers/base/dd.c | 9 +++++++++
>> drivers/base/dma-mapping.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> drivers/of/platform.c | 5 +----
>> drivers/pci/probe.c | 28 ----------------------------
>> include/linux/dma-mapping.h | 12 ++++++++++++
>> 6 files changed, 62 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>> index fb19e1c..c05f241 100644
>> --- a/drivers/acpi/glue.c
>> +++ b/drivers/acpi/glue.c
>> @@ -176,7 +176,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
>> struct list_head *physnode_list;
>> unsigned int node_id;
>> int retval = -EINVAL;
>> - enum dev_dma_attr attr;
>>
>> if (has_acpi_companion(dev)) {
>> if (acpi_dev) {
>> @@ -233,10 +232,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
>> if (!has_acpi_companion(dev))
>> ACPI_COMPANION_SET(dev, acpi_dev);
>>
>> - attr = acpi_get_dma_attr(acpi_dev);
>> - if (attr != DEV_DMA_NOT_SUPPORTED)
>> - acpi_dma_configure(dev, attr);
>> -
>> acpi_physnode_link_name(physical_node_name, node_id);
>> retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
>> physical_node_name);
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index a1fbf55..4882f06 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -19,6 +19,7 @@
>>
>> #include <linux/device.h>
>> #include <linux/delay.h>
>> +#include <linux/dma-mapping.h>
>> #include <linux/module.h>
>> #include <linux/kthread.h>
>> #include <linux/wait.h>
>> @@ -356,6 +357,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>> if (ret)
>> goto pinctrl_bind_failed;
>>
>> + ret = dma_configure(dev);
>> + if (ret)
>> + goto dma_failed;
>> +
>> if (driver_sysfs_add(dev)) {
>> printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
>> __func__, dev_name(dev));
>> @@ -417,6 +422,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>> goto done;
>>
>> probe_failed:
>> + dma_deconfigure(dev);
>> +dma_failed:
>> if (dev->bus)
>> blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>> BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
>> @@ -826,6 +833,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)
>> drv->remove(dev);
>>
>> device_links_driver_cleanup(dev);
>> + dma_deconfigure(dev);
>> +
>> devres_release_all(dev);
>> dev->driver = NULL;
>> dev_set_drvdata(dev, NULL);
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index efd71cf..449b948 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -7,9 +7,11 @@
>> * This file is released under the GPLv2.
>> */
>>
>> +#include <linux/acpi.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/export.h>
>> #include <linux/gfp.h>
>> +#include <linux/of_device.h>
>> #include <linux/slab.h>
>> #include <linux/vmalloc.h>
>>
>> @@ -341,3 +343,41 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
>> vunmap(cpu_addr);
>> }
>> #endif
>> +
>> +/*
>> + * Common configuration to enable DMA API use for a device
>> + */
>> +#include <linux/pci.h>
>> +
>> +int dma_configure(struct device *dev)
>> +{
>> + struct device *bridge = NULL, *dma_dev = dev;
>> + enum dev_dma_attr attr;
>> +
>> + if (dev_is_pci(dev)) {
>> + bridge = pci_get_host_bridge_device(to_pci_dev(dev));
>> + dma_dev = bridge;
>> + if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
>> + dma_dev->parent->of_node)
>> + dma_dev = dma_dev->parent;
>> + }
>> +
>> + if (dma_dev->of_node) {
>> + of_dma_configure(dev, dma_dev->of_node);
>> + } else if (has_acpi_companion(dma_dev)) {
>> + attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
>> + if (attr != DEV_DMA_NOT_SUPPORTED)
>> + acpi_dma_configure(dev, attr);
>> + }
>> +
>> + if (bridge)
>> + pci_put_host_bridge_device(bridge);
>> +
>> + return 0;
>> +}
>> +
>> +void dma_deconfigure(struct device *dev)
>> +{
>> + of_dma_deconfigure(dev);
>> + acpi_dma_deconfigure(dev);
>> +}
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index 5344db5..2aa4ebb 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -22,6 +22,7 @@
>> #include <linux/slab.h>
>> #include <linux/of_address.h>
>> #include <linux/of_device.h>
>> +#include <linux/of_iommu.h>
>> #include <linux/of_irq.h>
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> @@ -186,11 +187,9 @@ static struct platform_device *of_platform_device_create_pdata(
>>
>> dev->dev.bus = &platform_bus_type;
>> dev->dev.platform_data = platform_data;
>> - of_dma_configure(&dev->dev, dev->dev.of_node);
>> of_msi_configure(&dev->dev, dev->dev.of_node);
>>
>> if (of_device_add(dev) != 0) {
>> - of_dma_deconfigure(&dev->dev);
>> platform_device_put(dev);
>> goto err_clear_flag;
>> }
>> @@ -248,7 +247,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>> dev_set_name(&dev->dev, "%s", bus_id);
>> else
>> of_device_make_bus_id(&dev->dev);
>> - of_dma_configure(&dev->dev, dev->dev.of_node);
>>
>> /* Allow the HW Peripheral ID to be overridden */
>> prop = of_get_property(node, "arm,primecell-periphid", NULL);
>> @@ -542,7 +540,6 @@ static int of_platform_device_destroy(struct device *dev, void *data)
>> amba_device_unregister(to_amba_device(dev));
>> #endif
>>
>> - of_dma_deconfigure(dev);
>> of_node_clear_flag(dev->of_node, OF_POPULATED);
>> of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
>> return 0;
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index dfc9a27..5a8dd43 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1893,33 +1893,6 @@ static void pci_set_msi_domain(struct pci_dev *dev)
>> dev_set_msi_domain(&dev->dev, d);
>> }
>>
>> -/**
>> - * pci_dma_configure - Setup DMA configuration
>> - * @dev: ptr to pci_dev struct of the PCI device
>> - *
>> - * Function to update PCI devices's DMA configuration using the same
>> - * info from the OF node or ACPI node of host bridge's parent (if any).
>> - */
>> -static void pci_dma_configure(struct pci_dev *dev)
>> -{
>> - struct device *bridge = pci_get_host_bridge_device(dev);
>> -
>> - if (IS_ENABLED(CONFIG_OF) &&
>> - bridge->parent && bridge->parent->of_node) {
>> - of_dma_configure(&dev->dev, bridge->parent->of_node);
>> - } else if (has_acpi_companion(bridge)) {
>> - struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>> - enum dev_dma_attr attr = acpi_get_dma_attr(adev);
>> -
>> - if (attr == DEV_DMA_NOT_SUPPORTED)
>> - dev_warn(&dev->dev, "DMA not supported.\n");
>> - else
>> - acpi_dma_configure(&dev->dev, attr);
>> - }
>> -
>> - pci_put_host_bridge_device(bridge);
>> -}
>> -
>> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>> {
>> int ret;
>> @@ -1933,7 +1906,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>> dev->dev.dma_mask = &dev->dma_mask;
>> dev->dev.dma_parms = &dev->dma_parms;
>> dev->dev.coherent_dma_mask = 0xffffffffull;
>> - pci_dma_configure(dev);
>>
>> pci_set_dma_max_seg_size(dev, 65536);
>> pci_set_dma_seg_boundary(dev, 0xffffffff);
>> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
>> index 0977317..4f3eece 100644
>> --- a/include/linux/dma-mapping.h
>> +++ b/include/linux/dma-mapping.h
>> @@ -728,6 +728,18 @@ void *dma_mark_declared_memory_occupied(struct device *dev,
>> }
>> #endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */
>>
>> +#ifdef CONFIG_HAS_DMA
>> +int dma_configure(struct device *dev);
>> +void dma_deconfigure(struct device *dev);
>> +#else
>> +static inline int dma_configure(struct device *dev)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline void dma_deconfigure(struct device *dev) {}
>> +#endif
>> +
>> /*
>> * Managed DMA API
>> */
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ 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