* Re: [PATCH v4 1/2] mailbox: Add driver for Broadcom FlexRM ring manager
From: Anup Patel @ 2017-01-19 3:15 UTC (permalink / raw)
To: Jassi Brar, Rob Herring
Cc: Mark Rutland, Device Tree, Anup Patel, Scott Branden, Ray Jui,
Linux Kernel, Pramod KUMAR, BCM Kernel Feedback, Rob Rice,
Linux ARM Kernel
In-Reply-To: <1483614475-3442-2-git-send-email-anup.patel@broadcom.com>
Hi All,
Any comments on this patch ??
Regards,
Anup
^ permalink raw reply
* Re: [PATCH v11 11/12] drm/mediatek: update DSI sub driver flow for sending commands to panel
From: CK Hu @ 2017-01-19 3:20 UTC (permalink / raw)
To: YT Shen
Cc: Mark Rutland, devicetree, srv_heupstream, emil.l.velikov,
linux-kernel, dri-devel, Rob Herring, linux-mediatek,
Matthias Brugger, yingjoe.chen, shaoming chen, linux-arm-kernel
In-Reply-To: <1484117473-46644-12-git-send-email-yt.shen@mediatek.com>
Hi, YT:
one comment inline.
On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote:
> This patch update enable/disable flow of DSI module.
> Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> In this case: DSI -> panel, the DSI sub driver flow should be updated.
> We need to initialize DSI first so that we can send commands to panel.
>
> Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 89 +++++++++++++++++++++++++++++++-------
> 1 file changed, 74 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 85f22d2..21392c4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -126,6 +126,10 @@
> #define CLK_HS_POST (0xff << 8)
> #define CLK_HS_EXIT (0xff << 16)
>
> +#define DSI_VM_CMD_CON 0x130
> +#define VM_CMD_EN BIT(0)
> +#define TS_VFP_EN BIT(5)
> +
> #define DSI_CMDQ0 0x180
> #define CONFIG (0xff << 0)
> #define SHORT_PACKET 0
> @@ -365,16 +369,23 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
> u32 vid_mode = CMD_MODE;
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> - vid_mode = SYNC_PULSE_MODE;
> -
> - if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> + if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
> vid_mode = BURST_MODE;
> + else if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
> + vid_mode = SYNC_PULSE_MODE;
> + else
> + vid_mode = SYNC_EVENT_MODE;
> }
>
> writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
> }
>
> +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> +{
> + mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> + mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> +}
> +
> static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> {
> struct videomode *vm = &dsi->vm;
> @@ -512,6 +523,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> writel(1, dsi->regs + DSI_START);
> }
>
> +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> +{
> + writel(0, dsi->regs + DSI_START);
> +}
> +
> +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> +{
> + writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> +}
> +
> static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> {
> u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> @@ -570,6 +591,19 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
> +{
> + mtk_dsi_irq_data_clear(dsi, irq_flag);
> + mtk_dsi_set_cmd_mode(dsi);
> +
> + if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t)) {
> + DRM_ERROR("failed to switch cmd mode\n");
> + return -ETIME;
> + } else {
> + return 0;
> + }
> +}
> +
> static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> {
> if (WARN_ON(dsi->refcount == 0))
> @@ -578,6 +612,17 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> if (--dsi->refcount != 0)
> return;
>
> + mtk_dsi_stop(dsi);
> + if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
> + if (dsi->panel) {
> + if (drm_panel_unprepare(dsi->panel)) {
Why drm_panel_unprepare() in mtk_dsi_poweroff() but drm_panel_prepare()
in mtk_output_dsi_enable()? This asymmetric design would easily cause
some bugs.
Regards,
CK
> + DRM_ERROR("failed to unprepare the panel\n");
> + return;
> + }
> + }
> + }
> +
> + mtk_dsi_reset_engine(dsi);
> mtk_dsi_lane0_ulp_mode_enter(dsi);
> mtk_dsi_clk_ulp_mode_enter(dsi);
>
> @@ -596,13 +641,6 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> if (dsi->enabled)
> return;
>
> - if (dsi->panel) {
> - if (drm_panel_prepare(dsi->panel)) {
> - DRM_ERROR("failed to setup the panel\n");
> - return;
> - }
> - }
> -
> ret = mtk_dsi_poweron(dsi);
> if (ret < 0) {
> DRM_ERROR("failed to power on dsi\n");
> @@ -610,22 +648,43 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> }
>
> mtk_dsi_rxtx_control(dsi);
> + mtk_dsi_ps_control_vact(dsi);
> + mtk_dsi_set_vm_cmd(dsi);
> + mtk_dsi_config_vdo_timing(dsi);
> + mtk_dsi_set_interrupt_enable(dsi);
>
> mtk_dsi_clk_ulp_mode_leave(dsi);
> mtk_dsi_lane0_ulp_mode_leave(dsi);
> mtk_dsi_clk_hs_mode(dsi, 0);
> - mtk_dsi_set_mode(dsi);
>
> - mtk_dsi_ps_control_vact(dsi);
> - mtk_dsi_config_vdo_timing(dsi);
> - mtk_dsi_set_interrupt_enable(dsi);
> + if (dsi->panel) {
> + if (drm_panel_prepare(dsi->panel)) {
> + DRM_ERROR("failed to prepare the panel\n");
> + goto err_dsi_power_off;
> + }
> + }
>
> mtk_dsi_set_mode(dsi);
> mtk_dsi_clk_hs_mode(dsi, 1);
>
> mtk_dsi_start(dsi);
>
> + if (dsi->panel) {
> + if (drm_panel_enable(dsi->panel)) {
> + DRM_ERROR("failed to enable the panel\n");
> + goto err_panel_disable;
> + }
> + }
> +
> dsi->enabled = true;
> +
> + return;
> +err_panel_disable:
> + mtk_dsi_stop(dsi);
> + if (dsi->panel)
> + drm_panel_unprepare(dsi->panel);
> +err_dsi_power_off:
> + mtk_dsi_poweroff(dsi);
> }
>
> static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCHv4 2/5] arm: mvebu: support for SMP on 98DX3336 SoC
From: Chris Packham @ 2017-01-19 3:23 UTC (permalink / raw)
To: Stephen Boyd
Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, Jayachandran C,
Jason Cooper, Geert Uytterhoeven, devicetree@vger.kernel.org,
Russell King, Juri Lelli, linux-kernel@vger.kernel.org,
Rob Herring, Chris Brand, Sudeep Holla, Gregory Clement,
linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <20170119004750.GC4857@codeaurora.org>
On 19/01/17 13:48, Stephen Boyd wrote:
> On 01/13, Chris Packham wrote:
>> diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
>> index 46c742d3bd41..59be3ca0464f 100644
>> --- a/arch/arm/mach-mvebu/platsmp.c
>> +++ b/arch/arm/mach-mvebu/platsmp.c
>> @@ -184,3 +184,89 @@ const struct smp_operations armada_xp_smp_ops __initconst = {
>>
>> CPU_METHOD_OF_DECLARE(armada_xp_smp, "marvell,armada-xp-smp",
>> &armada_xp_smp_ops);
>> +
>> +struct resume_controller {
>> + u32 resume_control;
>> + u32 resume_boot_addr;
>> +};
>> +
>> +static const struct resume_controller mv98dx3336_resume_controller = {
>> + .resume_control = 0x08,
>> + .resume_boot_addr = 0x04,
>> +};
>> +
>> +static const struct of_device_id of_mv98dx3236_resume_table[] = {
>> + {
>> + .compatible = "marvell,98dx3336-resume-ctrl",
>> + .data = (void *)&mv98dx3336_resume_controller,
>
> Useless cast?
>
>> + },
>> + { /* end of list */ },
>> +};
>> +
>> +static int mv98dx3236_resume_set_cpu_boot_addr(int hw_cpu, void *boot_addr)
>> +{
>> + const struct of_device_id *match;
>> + struct device_node *np;
>> + void __iomem *base;
>> + struct resume_controller *rc;
>> +
>> + WARN_ON(hw_cpu != 1);
>> +
>> + np = of_find_matching_node_and_match(NULL, of_mv98dx3236_resume_table,
>> + &match);
>> + if (!np)
>> + return -ENODEV;
>> +
>> + base = of_io_request_and_map(np, 0, of_node_full_name(np));
>> + rc = (struct resume_controller *)match->data;
>
> Useless cast?
>
>> + of_node_put(np);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + writel(0, base + rc->resume_control);
>> + writel(virt_to_phys(boot_addr), base + rc->resume_boot_addr);
>> +
>
> Otherwise
>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>
Thanks for the review. Changes will be in v5.
^ permalink raw reply
* Re: [PATCHv4 1/5] clk: mvebu: support for 98DX3236 SoC
From: Chris Packham @ 2017-01-19 3:24 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree@vger.kernel.org, Michael Turquette,
Stephen Boyd, linux-kernel@vger.kernel.org,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170118222508.prn5xk63nuc3ocr7@rob-hp-laptop>
On 19/01/17 11:25, Rob Herring wrote:
> On Fri, Jan 13, 2017 at 10:12:16PM +1300, Chris Packham wrote:
>> The 98DX3236, 98DX3336, 98DX4521 and variants have a different TCLK from
>> the Armada XP (200MHz vs 250MHz). The CPU core clock is fixed at 800MHz.
>>
>> The clock gating options are a subset of those on the Armada XP.
>>
>> The core clock divider is different to the Armada XP also.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>> Changes in v2:
>> - Update devicetree binding documentation for new compatible string
>> Changes in v3:
>> - Add 98dx3236 support to mvebu/clk-corediv.c rather than creating a new
>> driver.
>> - Document mv98dx3236-corediv-clock binding
>> Changes in v4:
>> - None
>>
>> .../bindings/clock/mvebu-corediv-clock.txt | 1 +
>> .../devicetree/bindings/clock/mvebu-cpu-clock.txt | 1 +
>
> Please add acks when posting new versions.
>
> Acked-by: Rob Herring <robh@kernel.org>
>
Thanks Rob. I must have missed the earlier ack. Will be in v5.
^ permalink raw reply
* [PATCH] arm64: dts: msm8996: Add ADSP PIL node
From: Sarangdhar Joshi @ 2017-01-19 3:31 UTC (permalink / raw)
To: Andy Gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, Bjorn Andersson
Cc: Trilok Soni, devicetree, linux-arm-msm, Stephen Boyd,
linux-kernel, linux-soc, linux-arm-kernel
Add ADSP node required for Qualcomm ADSP Peripheral Image Loader.
Signed-off-by: Sarangdhar Joshi <spjoshi@codeaurora.org>
---
This patch uses "xo_board" clock for now. We would have to move to
rpmcc once it is available.
arch/arm64/boot/dts/qcom/msm8996.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index c9c7fd7..7cebf63 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -181,6 +181,26 @@
method = "smc";
};
+ adsp-pil {
+ compatible = "qcom,msm8996-adsp-pil";
+
+ interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>,
+ <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+ <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+ <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+ <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "wdog", "fatal", "ready",
+ "handover", "stop-ack";
+
+ clocks = <&xo_board>;
+ clock-names = "xo";
+
+ memory-region = <&adsp_region>;
+
+ qcom,smem-states = <&adsp_smp2p_out 0>;
+ qcom,smem-state-names = "stop";
+ };
+
adsp-smp2p {
compatible = "qcom,smp2p";
qcom,smem = <443>, <429>;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH v5] ARM64: dts: meson-gx: Add firmware reserved memory zones
From: Kevin Hilman @ 2017-01-19 4:36 UTC (permalink / raw)
To: Andreas Färber
Cc: Neil Armstrong, xypron.glpk-Mmb7MZpHnFY,
carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <eca803d3-1a67-cda5-554d-4313ae56635e-l3A5Bk7waGM@public.gmane.org>
Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org> writes:
> Am 19.01.2017 um 01:20 schrieb Andreas Färber:
>> Hi,
>>
>> Am 18.01.2017 um 17:50 schrieb Neil Armstrong:
>>> The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
>>> this patch adds these reserved zones.
>>>
>>> Without such reserved memory zones, running the following stress command :
>>> $ stress-ng --vm 16 --vm-bytes 128M --timeout 10s
>>> multiple times:
>>>
>>> Could lead to the following kernel crashes :
>>> [ 46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
>>> ...
>>> [ 47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
>>> ...
>>> Instead of the OOM killer.
>>>
>>
>> I miss a Fixes: or Cc: here for the backport you desired. To have it
>> fixed back to my very introduction:
>>
>> Fixes: 4f24eda8401f ("ARM64: dts: Prepare configs for Amlogic Meson GXBaby")
>>
>> People backporting it would need to handle the meson-{gx => gxbb}.dtsi
>> transition for 4.9 down to 4.6, which seems fairly straightforward.
>>
>>> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>> ---
>>> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 18 ++++++++++++++++++
>>> 1 file changed, 18 insertions(+)
>>>
>>> Changes since v4 at [5]:
>>> - Move start of ddr memory to reserved-memory node
>>> - Drop memory node move
>>> - Fix typo in sizes
>>>
>>> Changes since resent v2 at [4]:
>>> - Fix invalid comment of useable memory attributes
>>>
>>> Changes since original v2 at [3]:
>>> - Typo in commit 2GiB -> 1GiB, 4GiB -> 2GiB
>>>
>>> Changes since v2 at [2]:
>>> - Moved all memory node out of dtsi
>>> - Added comment about useable memory
>>> - Fixed comment about secmon reserved zone
>>>
>>> Changes since v1 at [1] :
>>> - Renamed reg into linux,usable-memory to ovveride u-boot memory
>>> - only kept secmon memory zone
>>>
>>> [1] http://lkml.kernel.org/r/20161212101801.28491-1-narmstrong@baylibre.com
>>> [2] http://lkml.kernel.org/r/1483105232-6242-1-git-send-email-narmstrong@baylibre.com
>>> [3] http://lkml.kernel.org/r/1484128128-22454-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
>>> [4] http://lkml.kernel.org/r/1484128540-22662-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
>>> [5] http://lkml.kernel.org/r/1484129414-23325-1-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
>>>
>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> index eada0b5..63d52b7 100644
>>> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>>> @@ -55,6 +55,24 @@
>>> #address-cells = <2>;
>>> #size-cells = <2>;
>>>
>>> + reserved-memory {
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + ranges;
>>> +
>>> + /* 16 MiB reserved for Hardware ROM Firmware */
>>> + hwrom: hwrom {
>>
>> Both sub-nodes get a label that is unused, but reserved-memory itself
>> does not (my v4 remark). Intentional?
>>
>>> + reg = <0x0 0x0 0x0 0x1000000>;
>>> + no-map;
>>> + };
>>> +
>>> + /* 2 MiB reserved for ARM Trusted Firmware (BL31) */
>>> + secmon: secmon {
>>
>> I note that this .dtsi further down has a node /firmware/secure-monitor
>> with label sm.
>> a) Is there any naming convention such as secmon_mem to adopt here to
>> avoid mixups with sm?
>> b) Should this secmon node be referenced in the secure-monitor node via
>> memory-node = <&secmon>; to model their connection, thereby giving the
>> label a use? Or should we maybe merge the two nodes by moving the
>> compatible string here?
>>
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>
> Answering my own question: the example labels use _reserved suffix.
>
>>> + reg = <0x0 0x10000000 0x0 0x200000>;
>
> And since we use a reg property here, the node name should get a unit
> address to avoid future dtc warnings/errors. Ditto for hwrom.
OK, I added Fixes:, your Reviewed-by, added the _reserved suffix and
unit address and applied to v4.10/fixes.
Update patch below for reference.
Other cleanups/fixups (like adding a phandle from secure monitor) can be
done as add-ons, as they are not strictly related to this fix.
Kevin
>From ecb88f3001ed9ee8c53450d971de8c18bcbf7925 Mon Sep 17 00:00:00 2001
From: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Date: Wed, 18 Jan 2017 17:50:45 +0100
Subject: [PATCH] ARM64: dts: meson-gx: Add firmware reserved memory zones
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
this patch adds these reserved zones.
Without such reserved memory zones, running the following stress command :
$ stress-ng --vm 16 --vm-bytes 128M --timeout 10s
multiple times:
Could lead to the following kernel crashes :
[ 46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
...
[ 47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
...
Instead of the OOM killer.
Fixes: 4f24eda8401f ("ARM64: dts: Prepare configs for Amlogic Meson GXBaby")
Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Reviewed-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
[khilman: added Fixes tag, added _reserved and unit addresses]
Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index eada0b58ba1c..0cbe24b49710 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -55,6 +55,24 @@
#address-cells = <2>;
#size-cells = <2>;
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ /* 16 MiB reserved for Hardware ROM Firmware */
+ hwrom_reserved: hwrom@0 {
+ reg = <0x0 0x0 0x0 0x1000000>;
+ no-map;
+ };
+
+ /* 2 MiB reserved for ARM Trusted Firmware (BL31) */
+ secmon_reserved: secmon@10000000 {
+ reg = <0x0 0x10000000 0x0 0x200000>;
+ no-map;
+ };
+ };
+
cpus {
#address-cells = <0x2>;
#size-cells = <0x0>;
--
2.9.3
--
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 v4 1/2] power: reset: add linkstation-reset driver
From: Sebastian Reichel @ 2017-01-19 4:43 UTC (permalink / raw)
To: Roger Shimizu
Cc: Rob Herring, Andrew Lunn, Florian Fainelli, Ryan Tandy, linux-pm,
Herbert Valerio Riedel, devicetree, Martin Michlmayr,
linux-arm-kernel, Sylver Bruneau
In-Reply-To: <CAEQ9gEkoKc0Yv+kLYGWgHZZW92u0tW8E9U0eT7ZhAx79yKxzLQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --]
Hi Roger,
On Wed, Jan 18, 2017 at 09:08:13PM +0900, Roger Shimizu wrote:
> On Sun, Jan 8, 2017 at 12:04 AM, Roger Shimizu <rogershimizu@gmail.com> wrote:
> > Buffalo Linkstation / KuroBox and their variants need magic command
> > sending to UART1 to power-off.
> >
> > Power driver linkstation-reset implements the magic command and I/O
> > routine, which come from files listed below:
> > - arch/arm/mach-orion5x/kurobox_pro-setup.c
> > - arch/arm/mach-orion5x/terastation_pro2-setup.c
>
> I think there's not much concern regarding to this series.
> Could you kindly help to apply this patch?
Well you dropped the DT binding, but still introduce new DT
properties. Since they are not documented without the binding
I won't merge this. DT binding documentation is not optional.
In other words: NAK on the binding effectively means NAK on
the driver in its current state.
If you want to see this merged rebase it to Rob's generic
serial bindings [0] and help to support him getting everything
into mainline ASAP. Currently the feedback seems to be quite
positive, so I hope to see it merged for 4.11.
[0] https://lwn.net/Articles/711794/ (current proposal from 2 days ago)
-- Sebastian
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy
From: Vivek Gautam @ 2017-01-19 5:12 UTC (permalink / raw)
To: Stephen Boyd, Bjorn Andersson
Cc: Kishon Vijay Abraham I, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170119004028.GA4857-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 01/19/2017 06:10 AM, Stephen Boyd wrote:
> On 01/18, Bjorn Andersson wrote:
>> On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
>>> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> [..]
>>>>> + reset-names = "phy", "common", "cfg",
>>>>> + "lane0", "lane1", "lane2";
>>>> Each lane has a separate clock, separate reset.. why not create sub-nodes for
>>>> each lane?
>>> Yes, each lane has separate pipe clock and resets.
>>> I can have a binding such as written below.
>> +1
>>
>>> Does it makes sense to pull in the tx, rx and pcs offsets as well
>>> to the child node, and iomap the entire address space of the phy ?
>>>
>> Note that you don't have to follow the same structure in your device
>> driver as you describe your hardware in devicetree.
>>
>> I would suggest that you replace the lane-offset and various lane
>> specific resources with subnodes, but keep the driver "as is".
>>
> Didn't we already move away from subnodes for lanes in an earlier
> revision of these patches? I seem to recall we did that because
> lanes are not devices and the whole "phy as a bus" concept not
> making sense.
Yea, we started out without having any sub-nodes and we
argued that we don't require them since the qmp device is
represented by the qmp node itself.
The lanes otoh are representative of gen_phys and related properties.
In the driver -
"struct qmp_phy " represents the lanes and holds "struct phy",
"struct qcom_qmp" represents the qmp block as a whole and holds "struct
device"
Does this make lanes qualify to be childs of qmp ?
"phy as a bus" (just trying to understand here) -
let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
So, should this phy controller be a bus providing two ports (and so we
will have
couple of child nodes to the phy controller) ?
Regards
Vivek
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 v11 12/12] drm/mediatek: add support for Mediatek SoC MT2701
From: CK Hu @ 2017-01-19 5:30 UTC (permalink / raw)
To: YT Shen
Cc: Mark Rutland, devicetree, srv_heupstream, emil.l.velikov,
linux-kernel, dri-devel, Rob Herring, linux-mediatek,
Matthias Brugger, yingjoe.chen, linux-arm-kernel
In-Reply-To: <1484117473-46644-13-git-send-email-yt.shen@mediatek.com>
Hi, YT:
On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote:
> This patch add support for the Mediatek MT2701 DISP subsystem.
> There is only one OVL engine in MT2701.
>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
Acked-by: CK Hu <ck.hu@mediatek.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 8 ++++++++
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 ++++++
> drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 17 +++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 7 +++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 29 +++++++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_dsi.c | 1 +
> drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 6 ++++++
> 7 files changed, 74 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 4552178..a14d7d6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -35,6 +35,7 @@
> #define DISP_REG_OVL_PITCH(n) (0x0044 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_CTRL(n) (0x00c0 + 0x20 * (n))
> #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n))
> +#define DISP_REG_OVL_ADDR_MT2701 0x0040
> #define DISP_REG_OVL_ADDR_MT8173 0x0f40
> #define DISP_REG_OVL_ADDR(ovl, n) ((ovl)->data->addr + 0x20 * (n))
>
> @@ -303,12 +304,19 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct mtk_disp_ovl_data mt2701_ovl_driver_data = {
> + .addr = DISP_REG_OVL_ADDR_MT2701,
> + .fmt_rgb565_is_0 = false,
> +};
> +
> static const struct mtk_disp_ovl_data mt8173_ovl_driver_data = {
> .addr = DISP_REG_OVL_ADDR_MT8173,
> .fmt_rgb565_is_0 = true,
> };
>
> static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
> + { .compatible = "mediatek,mt2701-disp-ovl",
> + .data = &mt2701_ovl_driver_data},
> { .compatible = "mediatek,mt8173-disp-ovl",
> .data = &mt8173_ovl_driver_data},
> {},
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index e5e5318..b68a513 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -236,11 +236,17 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct mtk_disp_rdma_data mt2701_rdma_driver_data = {
> + .fifo_size = SZ_4K,
> +};
> +
> static const struct mtk_disp_rdma_data mt8173_rdma_driver_data = {
> .fifo_size = SZ_8K,
> };
>
> static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
> + { .compatible = "mediatek,mt2701-disp-rdma",
> + .data = &mt2701_rdma_driver_data},
> { .compatible = "mediatek,mt8173-disp-rdma",
> .data = &mt8173_rdma_driver_data},
> {},
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> index a9b209c..8130f3d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
> @@ -60,6 +60,13 @@
> #define MT8173_MUTEX_MOD_DISP_PWM1 BIT(24)
> #define MT8173_MUTEX_MOD_DISP_OD BIT(25)
>
> +#define MT2701_MUTEX_MOD_DISP_OVL BIT(3)
> +#define MT2701_MUTEX_MOD_DISP_WDMA BIT(6)
> +#define MT2701_MUTEX_MOD_DISP_COLOR BIT(7)
> +#define MT2701_MUTEX_MOD_DISP_BLS BIT(9)
> +#define MT2701_MUTEX_MOD_DISP_RDMA0 BIT(10)
> +#define MT2701_MUTEX_MOD_DISP_RDMA1 BIT(12)
> +
> #define MUTEX_SOF_SINGLE_MODE 0
> #define MUTEX_SOF_DSI0 1
> #define MUTEX_SOF_DSI1 2
> @@ -92,6 +99,15 @@ struct mtk_ddp {
> const unsigned int *mutex_mod;
> };
>
> +static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> + [DDP_COMPONENT_BLS] = MT2701_MUTEX_MOD_DISP_BLS,
> + [DDP_COMPONENT_COLOR0] = MT2701_MUTEX_MOD_DISP_COLOR,
> + [DDP_COMPONENT_OVL0] = MT2701_MUTEX_MOD_DISP_OVL,
> + [DDP_COMPONENT_RDMA0] = MT2701_MUTEX_MOD_DISP_RDMA0,
> + [DDP_COMPONENT_RDMA1] = MT2701_MUTEX_MOD_DISP_RDMA1,
> + [DDP_COMPONENT_WDMA0] = MT2701_MUTEX_MOD_DISP_WDMA,
> +};
> +
> static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> [DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
> [DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
> @@ -390,6 +406,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id ddp_driver_dt_match[] = {
> + { .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod},
> { .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
> {},
> };
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index f6e853a..8b52416 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -39,6 +39,7 @@
> #define DISP_REG_UFO_START 0x0000
>
> #define DISP_COLOR_CFG_MAIN 0x0400
> +#define DISP_COLOR_START_MT2701 0x0f00
> #define DISP_COLOR_START_MT8173 0x0c00
> #define DISP_COLOR_START(comp) ((comp)->data->color_offset)
> #define DISP_COLOR_WIDTH(comp) (DISP_COLOR_START(comp) + 0x50)
> @@ -285,11 +286,17 @@ struct mtk_ddp_comp_match {
> [DDP_COMPONENT_WDMA1] = { MTK_DISP_WDMA, 1, NULL },
> };
>
> +static const struct mtk_disp_color_data mt2701_color_driver_data = {
> + .color_offset = DISP_COLOR_START_MT2701,
> +};
> +
> static const struct mtk_disp_color_data mt8173_color_driver_data = {
> .color_offset = DISP_COLOR_START_MT8173,
> };
>
> static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
> + { .compatible = "mediatek,mt2701-disp-color",
> + .data = &mt2701_color_driver_data},
> { .compatible = "mediatek,mt8173-disp-color",
> .data = &mt8173_color_driver_data},
> {},
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 074fe31..7daabae 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -128,6 +128,19 @@ static int mtk_atomic_commit(struct drm_device *drm,
> .atomic_commit = mtk_atomic_commit,
> };
>
> +static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
> + DDP_COMPONENT_OVL0,
> + DDP_COMPONENT_RDMA0,
> + DDP_COMPONENT_COLOR0,
> + DDP_COMPONENT_BLS,
> + DDP_COMPONENT_DSI0,
> +};
> +
> +static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
> + DDP_COMPONENT_RDMA1,
> + DDP_COMPONENT_DPI0,
> +};
> +
> static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
> DDP_COMPONENT_OVL0,
> DDP_COMPONENT_COLOR0,
> @@ -147,6 +160,14 @@ static int mtk_atomic_commit(struct drm_device *drm,
> DDP_COMPONENT_DPI0,
> };
>
> +static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> + .main_path = mt2701_mtk_ddp_main,
> + .main_len = ARRAY_SIZE(mt2701_mtk_ddp_main),
> + .ext_path = mt2701_mtk_ddp_ext,
> + .ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
> + .shadow_register = true,
> +};
> +
> static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> .main_path = mt8173_mtk_ddp_main,
> .main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
> @@ -340,16 +361,22 @@ static void mtk_drm_unbind(struct device *dev)
> };
>
> static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> + { .compatible = "mediatek,mt2701-disp-ovl", .data = (void *)MTK_DISP_OVL },
> { .compatible = "mediatek,mt8173-disp-ovl", .data = (void *)MTK_DISP_OVL },
> + { .compatible = "mediatek,mt2701-disp-rdma", .data = (void *)MTK_DISP_RDMA },
> { .compatible = "mediatek,mt8173-disp-rdma", .data = (void *)MTK_DISP_RDMA },
> { .compatible = "mediatek,mt8173-disp-wdma", .data = (void *)MTK_DISP_WDMA },
> + { .compatible = "mediatek,mt2701-disp-color", .data = (void *)MTK_DISP_COLOR },
> { .compatible = "mediatek,mt8173-disp-color", .data = (void *)MTK_DISP_COLOR },
> { .compatible = "mediatek,mt8173-disp-aal", .data = (void *)MTK_DISP_AAL},
> { .compatible = "mediatek,mt8173-disp-gamma", .data = (void *)MTK_DISP_GAMMA, },
> { .compatible = "mediatek,mt8173-disp-ufoe", .data = (void *)MTK_DISP_UFOE },
> + { .compatible = "mediatek,mt2701-dsi", .data = (void *)MTK_DSI },
> { .compatible = "mediatek,mt8173-dsi", .data = (void *)MTK_DSI },
> { .compatible = "mediatek,mt8173-dpi", .data = (void *)MTK_DPI },
> + { .compatible = "mediatek,mt2701-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
> { .compatible = "mediatek,mt8173-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
> + { .compatible = "mediatek,mt2701-disp-pwm", .data = (void *)MTK_DISP_BLS },
> { .compatible = "mediatek,mt8173-disp-pwm", .data = (void *)MTK_DISP_PWM },
> { .compatible = "mediatek,mt8173-disp-od", .data = (void *)MTK_DISP_OD },
> { }
> @@ -523,6 +550,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
> mtk_drm_sys_resume);
>
> static const struct of_device_id mtk_drm_of_ids[] = {
> + { .compatible = "mediatek,mt2701-mmsys",
> + .data = &mt2701_mmsys_driver_data},
> { .compatible = "mediatek,mt8173-mmsys",
> .data = &mt8173_mmsys_driver_data},
> { }
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 21392c4..e1832ea 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1228,6 +1228,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id mtk_dsi_of_match[] = {
> + { .compatible = "mediatek,mt2701-dsi" },
> { .compatible = "mediatek,mt8173-dsi" },
> { },
> };
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index fd84914..90e9131 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -465,11 +465,17 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct mtk_mipitx_data mt2701_mipitx_data = {
> + .mppll_preserve = (3 << 8)
> +};
> +
> static const struct mtk_mipitx_data mt8173_mipitx_data = {
> .mppll_preserve = (0 << 8)
> };
>
> static const struct of_device_id mtk_mipi_tx_match[] = {
> + { .compatible = "mediatek,mt2701-mipi-tx",
> + .data = &mt2701_mipitx_data },
> { .compatible = "mediatek,mt8173-mipi-tx",
> .data = &mt8173_mipitx_data },
> {},
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Question about OF-graph ports
From: Kuninori Morimoto @ 2017-01-19 6:07 UTC (permalink / raw)
To: Rob Herring, Mark Brown; +Cc: Linux-DT, Linux-ALSA
Hi Rob, Mark
and ALSA SoC ML
I'm working for OF-graph base sound card support.
Now I want to know 1 things which can't solve by myself.
It is OF-graph ports
ALSA SoC needs CPU, Codec, and Sound Card.
If CPU <-> Card <-> Codec case, OF-graph will be
card {
ports {
port@0 {
card_in: endpoint {
remote-endpoint = <&cpu_out>;
};
};
port@1 {
card_out: endpoint {
remote-endpoint = <&codec_in>;
};
};
};
};
cpu {
port {
cpu_out: endpoint {
remote-endpoint = <&card_in>;
};
};
};
codec {
port {
codec_in: endpoint {
remote-endpoint = <&card_out>;
};
};
};
This is OK.
If CPU has many ports, then ALSA SoC requests 1 Card which has many
DAIs (= Digital Audio Interface).
This case, cpu will have "ports" and many "port", this is OK.
SoC.0 Codec0
SoC.1 <-> Card <-> Codec1
SoC.2 Codec2
In "card", CPU/Codec pair is indicated by "ports" now
(= port0 is CPU, port1 is Codec, etc)
My question is in this case, how to know CPU/Codec pair on "card" ?
Can we have *multi ports*, like this ?
card {
ports {
ports@0 {
port@0 { /* SoC.0 */ };
port@1 { /* Codec0*/ };
};
ports@1 {
port@0 { /* SoC.1 */ };
port@1 { /* Codec1*/ };
};
ports@2 {
port@0 { /* SoC.2 */ };
port@1 { /* Codec2*/ };
};
};
};
Or can I use grouping, like this ?
card {
group = <port@0 port@1>,
<port@2 port@3>,
<port@4 port@5>;
ports {
port@0 { /* SoC.0 */ };
port@1 { /* SoC.1 */ };
port@2 { /* SoC.2 */ };
port@3 { /* Codec0*/ };
port@4 { /* Codec1*/ };
port@5 { /* Codec2*/ };
};
};
Best regards
---
Kuninori Morimoto
--
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 0/6] of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:31 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
Hi Rob
Now many driver is getting remote-endpoint by manually,
but we should use same method to get same result IMO.
Thus this patch adds of_graph_get_remote_endpoint() for this purpose.
And use it on several drivers
Kuninori Morimoto (6):
of_graph: add of_graph_get_remote_endpoint()
drm/omapdrm: use of_graph_get_remote_endpoint()
drm: rcar-du: use of_graph_get_remote_endpoint()
media: ti-vpe: cal: use of_graph_get_remote_endpoint()
v4l: of: use of_graph_get_remote_endpoint()
omapfb: use of_graph_get_remote_endpoint()
drivers/gpu/drm/omapdrm/dss/dss-of.c | 3 ++-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
drivers/media/platform/ti-vpe/cal.c | 2 +-
drivers/media/v4l2-core/v4l2-of.c | 3 ++-
drivers/of/base.c | 18 ++++++++++++++++--
drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 3 ++-
include/linux/of_graph.h | 8 ++++++++
7 files changed, 32 insertions(+), 7 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH v6 03/25] usb: ulpi: Support device discovery via DT
From: Peter Chen @ 2017-01-19 6:33 UTC (permalink / raw)
To: Stephen Boyd, Greg Kroah-Hartman
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi, Arnd Bergmann,
Neil Armstrong, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bjorn Andersson,
devicetree-u79uwXL29TY76Z2rM5mHXA, Peter Chen, Andy Gross,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161228225711.698-4-stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Wed, Dec 28, 2016 at 02:56:49PM -0800, Stephen Boyd wrote:
> The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
> product ID registers. This makes it impossible to make a ULPI
> driver match against the ID registers. Add support to discover
> the ULPI phys via DT help alleviate this problem. In the DT case,
> we'll look for a ULPI bus node underneath the device registering
> the ULPI viewport (or the parent of that device to support
> chipidea's device layout) and then match up the phy node
> underneath that with the ULPI device that's created.
>
> The side benefit of this is that we can use standard properties
> in the phy node like clks, regulators, gpios, etc. because we
> don't have firmware like ACPI to turn these things on for us. And
> we can use the DT phy binding to point our phy consumer to the
> phy provider.
>
> The ULPI bus code supports native enumeration by reading the
> vendor ID and product ID registers at device creation time, but
> we can't be certain that those register reads will succeed if the
> phy is not powered up. To avoid any problems with reading the ID
> registers before the phy is powered we fallback to DT matching
> when the ID reads fail.
>
> If the ULPI spec had some generic power sequencing for these
> registers we could put that into the ULPI bus layer and power up
> the device before reading the ID registers. Unfortunately this
> doesn't exist and the power sequence is usually device specific.
> By having the device matched up with DT we can avoid this
> problem.
>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Acked-by: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Greg, is it ok I pick up this patch, and send it with chipidea
changes together for 4.11-rc1 later?
Peter
> ---
> Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++
> drivers/usb/common/ulpi.c | 79 ++++++++++++++++++++++++--
> 2 files changed, 93 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
> new file mode 100644
> index 000000000000..ca179dc4bd50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> @@ -0,0 +1,20 @@
> +ULPI bus binding
> +----------------
> +
> +Phys that are behind a ULPI connection can be described with the following
> +binding. The host controller shall have a "ulpi" named node as a child, and
> +that node shall have one enabled node underneath it representing the ulpi
> +device on the bus.
> +
> +EXAMPLE
> +-------
> +
> +usb {
> + compatible = "vendor,usb-controller";
> +
> + ulpi {
> + phy {
> + compatible = "vendor,phy";
> + };
> + };
> +};
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 8b317702d761..c9480d77810c 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -16,6 +16,9 @@
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/clk/clk-conf.h>
>
> /* -------------------------------------------------------------------------- */
>
> @@ -39,6 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
> struct ulpi *ulpi = to_ulpi_dev(dev);
> const struct ulpi_device_id *id;
>
> + /* Some ULPI devices don't have a vendor id so rely on OF match */
> + if (ulpi->id.vendor == 0)
> + return of_driver_match_device(dev, driver);
> +
> for (id = drv->id_table; id->vendor; id++)
> if (id->vendor == ulpi->id.vendor &&
> id->product == ulpi->id.product)
> @@ -50,6 +57,11 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
> static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> struct ulpi *ulpi = to_ulpi_dev(dev);
> + int ret;
> +
> + ret = of_device_uevent_modalias(dev, env);
> + if (ret != -ENODEV)
> + return ret;
>
> if (add_uevent_var(env, "MODALIAS=ulpi:v%04xp%04x",
> ulpi->id.vendor, ulpi->id.product))
> @@ -60,6 +72,11 @@ static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
> static int ulpi_probe(struct device *dev)
> {
> struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
> + int ret;
> +
> + ret = of_clk_set_defaults(dev->of_node, false);
> + if (ret < 0)
> + return ret;
>
> return drv->probe(to_ulpi_dev(dev));
> }
> @@ -87,8 +104,13 @@ static struct bus_type ulpi_bus = {
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> + int len;
> struct ulpi *ulpi = to_ulpi_dev(dev);
>
> + len = of_device_get_modalias(dev, buf, PAGE_SIZE - 1);
> + if (len != -ENODEV)
> + return len;
> +
> return sprintf(buf, "ulpi:v%04xp%04x\n",
> ulpi->id.vendor, ulpi->id.product);
> }
> @@ -153,23 +175,45 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
>
> /* -------------------------------------------------------------------------- */
>
> -static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +static int ulpi_of_register(struct ulpi *ulpi)
> {
> - int ret;
> + struct device_node *np = NULL, *child;
> + struct device *parent;
> +
> + /* Find a ulpi bus underneath the parent or the grandparent */
> + parent = ulpi->dev.parent;
> + if (parent->of_node)
> + np = of_find_node_by_name(parent->of_node, "ulpi");
> + else if (parent->parent && parent->parent->of_node)
> + np = of_find_node_by_name(parent->parent->of_node, "ulpi");
> + if (!np)
> + return 0;
> +
> + child = of_get_next_available_child(np, NULL);
> + of_node_put(np);
> + if (!child)
> + return -EINVAL;
>
> - ulpi->dev.parent = dev; /* needed early for ops */
> + ulpi->dev.of_node = child;
> +
> + return 0;
> +}
> +
> +static int ulpi_read_id(struct ulpi *ulpi)
> +{
> + int ret;
>
> /* Test the interface */
> ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
> if (ret < 0)
> - return ret;
> + goto err;
>
> ret = ulpi_read(ulpi, ULPI_SCRATCH);
> if (ret < 0)
> return ret;
>
> if (ret != 0xaa)
> - return -ENODEV;
> + goto err;
>
> ulpi->id.vendor = ulpi_read(ulpi, ULPI_VENDOR_ID_LOW);
> ulpi->id.vendor |= ulpi_read(ulpi, ULPI_VENDOR_ID_HIGH) << 8;
> @@ -177,13 +221,35 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
> ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
>
> + /* Some ULPI devices don't have a vendor id so rely on OF match */
> + if (ulpi->id.vendor == 0)
> + goto err;
> +
> + request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
> + return 0;
> +err:
> + of_device_request_module(&ulpi->dev);
> + return 0;
> +}
> +
> +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +{
> + int ret;
> +
> + ulpi->dev.parent = dev; /* needed early for ops */
> ulpi->dev.bus = &ulpi_bus;
> ulpi->dev.type = &ulpi_dev_type;
> dev_set_name(&ulpi->dev, "%s.ulpi", dev_name(dev));
>
> ACPI_COMPANION_SET(&ulpi->dev, ACPI_COMPANION(dev));
>
> - request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
> + ret = ulpi_of_register(ulpi);
> + if (ret)
> + return ret;
> +
> + ret = ulpi_read_id(ulpi);
> + if (ret)
> + return ret;
>
> ret = device_register(&ulpi->dev);
> if (ret)
> @@ -234,6 +300,7 @@ EXPORT_SYMBOL_GPL(ulpi_register_interface);
> */
> void ulpi_unregister_interface(struct ulpi *ulpi)
> {
> + of_node_put(ulpi->dev.of_node);
> device_unregister(&ulpi->dev);
> }
> EXPORT_SYMBOL_GPL(ulpi_unregister_interface);
> --
> 2.10.0.297.gf6727b0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Best Regards,
Peter Chen
--
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 1/6] of_graph: add of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:34 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx@renesas.com>
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
It should use same method to get same result.
To getting remote-endpoint node,
let's use of_graph_get_remote_endpoint()
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
drivers/of/base.c | 18 ++++++++++++++++--
include/linux/of_graph.h | 8 ++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..5391a9b5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2426,6 +2426,20 @@ struct device_node *of_graph_get_endpoint_by_regs(
EXPORT_SYMBOL(of_graph_get_endpoint_by_regs);
/**
+ * of_graph_get_remote_endpoint() - get remote endpoint node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote endpoint node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_endpoint(const struct device_node *node)
+{
+ /* Get remote endpoint node. */
+ return of_parse_phandle(node, "remote-endpoint", 0);
+}
+EXPORT_SYMBOL(of_graph_get_remote_endpoint);
+
+/**
* of_graph_get_remote_port_parent() - get remote port's parent node
* @node: pointer to a local endpoint device_node
*
@@ -2439,7 +2453,7 @@ struct device_node *of_graph_get_remote_port_parent(
unsigned int depth;
/* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
/* Walk 3 levels up only if there is 'ports' node. */
for (depth = 3; depth && np; depth--) {
@@ -2463,7 +2477,7 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node)
struct device_node *np;
/* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np)
return NULL;
return of_get_next_parent(np);
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index bb3a5a2..d9d6d9c 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h
@@ -48,6 +48,8 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
struct device_node *previous);
struct device_node *of_graph_get_endpoint_by_regs(
const struct device_node *parent, int port_reg, int reg);
+struct device_node *of_graph_get_remote_endpoint(
+ const struct device_node *node);
struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node);
struct device_node *of_graph_get_remote_port(const struct device_node *node);
@@ -78,6 +80,12 @@ static inline struct device_node *of_graph_get_endpoint_by_regs(
return NULL;
}
+static inline struct device_node *of_graph_get_remote_endpoint(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
static inline struct device_node *of_graph_get_remote_port_parent(
const struct device_node *node)
{
--
1.9.1
^ permalink raw reply related
* [PATCH 2/6] drm/omapdrm: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:34 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
drivers/gpu/drm/omapdrm/dss/dss-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dss-of.c b/drivers/gpu/drm/omapdrm/dss/dss-of.c
index dfd4e96..f49f2ba 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss-of.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss-of.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/seq_file.h>
#include "omapdss.h"
@@ -127,7 +128,7 @@ static struct device_node *omapdss_of_get_remote_port(const struct device_node *
{
struct device_node *np;
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np)
return NULL;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/6] drm/omapdrm: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:36 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
drivers/gpu/drm/omapdrm/dss/dss-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dss-of.c b/drivers/gpu/drm/omapdrm/dss/dss-of.c
index dfd4e96..f49f2ba 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss-of.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss-of.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/seq_file.h>
#include "omapdss.h"
@@ -127,7 +128,7 @@ static struct device_node *omapdss_of_get_remote_port(const struct device_node *
{
struct device_node *np;
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np)
return NULL;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/6] drm: rcar-du: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:36 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx@renesas.com>
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 7a6bd8b..2ba728b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -391,7 +391,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu,
return -ENODEV;
}
- entity_ep_node = of_parse_phandle(ep->local_node, "remote-endpoint", 0);
+ entity_ep_node = of_graph_get_remote_endpoint(ep->local_node);
for_each_endpoint_of_node(entity, ep_node) {
if (ep_node == entity_ep_node)
--
1.9.1
^ permalink raw reply related
* [PATCH 4/6] media: ti-vpe: cal: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:37 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
From: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
drivers/media/platform/ti-vpe/cal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 7a058b6..ebf11b4 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1701,7 +1701,7 @@ static int of_cal_create_instance(struct cal_ctx *ctx, int inst)
asd->match_type = V4L2_ASYNC_MATCH_OF;
asd->match.of.node = sensor_node;
- remote_ep = of_parse_phandle(ep_node, "remote-endpoint", 0);
+ remote_ep = of_graph_get_remote_endpoint(ep_node);
if (!remote_ep) {
ctx_dbg(3, ctx, "can't get remote-endpoint\n");
goto cleanup_exit;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 5/6] v4l: of: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:37 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx@renesas.com>
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
drivers/media/v4l2-core/v4l2-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 93b3368..b2ed4b3 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -14,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/types.h>
@@ -284,7 +285,7 @@ int v4l2_of_parse_link(const struct device_node *node,
np = of_get_next_parent(np);
link->local_node = np;
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np) {
of_node_put(link->local_node);
return -ENOLINK;
--
1.9.1
^ permalink raw reply related
* [PATCH 6/6] omapfb: use of_graph_get_remote_endpoint()
From: Kuninori Morimoto @ 2017-01-19 6:38 UTC (permalink / raw)
To: Rob Herring, Frank Rowand
Cc: Linux-Kernel, Linux-DT, Laurent, Tomi Valkeinen, David Airlie,
Sean Paul, Peter Chen, Peter Ujfalusi, Benoit Parrot,
Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus,
Bartlomiej Zolnierkiewicz
In-Reply-To: <87o9z34l3o.wl%kuninori.morimoto.gx@renesas.com>
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Now, we can use of_graph_get_remote_endpoint(). Let's use it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
drivers/video/fbdev/omap2/omapfb/dss/dss-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
index d356a25..f1eb8b0 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dss-of.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/seq_file.h>
#include <video/omapfb_dss.h>
@@ -128,7 +129,7 @@ static struct device_node *omapdss_of_get_remote_port(const struct device_node *
{
struct device_node *np;
- np = of_parse_phandle(node, "remote-endpoint", 0);
+ np = of_graph_get_remote_endpoint(node);
if (!np)
return NULL;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 00/13] Ingenic JZ4740 / JZ4780 pinctrl driver
From: Linus Walleij @ 2017-01-19 6:38 UTC (permalink / raw)
To: Paul Cercueil
Cc: Rob Herring, Mark Rutland, Ralf Baechle, Ulf Hansson,
Boris Brezillon, Thierry Reding, Bartlomiej Zolnierkiewicz,
Maarten ter Huurne, Lars-Peter Clausen, Paul Burton,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Linux MIPS,
linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, linux-p
In-Reply-To: <20170117231421.16310-1-paul@crapouillou.net>
On Wed, Jan 18, 2017 at 12:14 AM, Paul Cercueil <paul@crapouillou.net> wrote:
> One problem still unresolved: the pinctrl framework does not allow us to
> configure each pin on demand (someone please prove me wrong), when the
> various PWM channels are requested or released. For instance, the PWM
> channels can be configured from sysfs, which would require all PWM pins
> to be configured properly beforehand for the PWM function, eventually
> causing conflicts with other platform or board drivers.
Why do you think this?
- Pincontrol handles can be obtained at runtime.
- Pincontrol states can be changed at runtime.
The fact that a the handle is retrived by the device core and set to
the states named "init" or "default" during boot is just a convenience.
You can have as many and as fine-grained states as you want. They
can pertain to just one pin too.
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 2/3] input: tm2-touchkey: Add touchkey driver support for TM2
From: Krzysztof Kozlowski @ 2017-01-19 7:17 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jaechul Lee, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, Kukjin Kim, Javier Martinez Canillas, Andi Shyti,
Chanwoo Choi, Beomho Seo, 이재철,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, lkml,
linux-samsung-soc@vger.kernel.org
In-Reply-To: <20170118224711.GB11881@dtor-ws>
On Thu, Jan 19, 2017 at 12:47 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Jan 17, 2017 at 08:10:56PM +0200, Krzysztof Kozlowski wrote:
>> On Tue, Jan 17, 2017 at 10:06:27AM -0800, Dmitry Torokhov wrote:
>> > On Tue, Jan 17, 2017 at 9:20 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> > > On Tue, Jan 17, 2017 at 02:54:38PM +0900, Jaechul Lee wrote:
>> > >> This patch adds support for the TM2 touch key and led
>> > >> functionality.
>> > >>
>> > >> The driver interfaces with userspace through an input device and
>> > >> reports KEY_PHONE and KEY_BACK event types. LED brightness can be
>> > >> controlled by "/sys/class/leds/tm2-touchkey/brightness".
>> > >>
>> > >> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>> > >> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
>> > >> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> > >> Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
>> > >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>> > >> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>> > >> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
>> > >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > >
>> > > This looks unusual. How did Dmitry's Sob end here?
>> >
>> > I sent Jaechul a version of the patch to try out.
>>
>> Ah, makes sense then.
>
> I picked up (and folded) the binding doc and driver patches, DTS should
> go through some other tree I believe.
Yes, I will take it. I waited for driver and bindings to be accepted.
Thanks,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 03/25] usb: ulpi: Support device discovery via DT
From: Greg Kroah-Hartman @ 2017-01-19 7:39 UTC (permalink / raw)
To: Peter Chen
Cc: Stephen Boyd, linux-usb, Felipe Balbi, Arnd Bergmann,
Neil Armstrong, linux-arm-msm, linux-kernel, Bjorn Andersson,
devicetree, Peter Chen, Andy Gross, linux-arm-kernel
In-Reply-To: <20170119063349.GB10621@b29397-desktop>
On Thu, Jan 19, 2017 at 02:33:49PM +0800, Peter Chen wrote:
> On Wed, Dec 28, 2016 at 02:56:49PM -0800, Stephen Boyd wrote:
> > The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
> > product ID registers. This makes it impossible to make a ULPI
> > driver match against the ID registers. Add support to discover
> > the ULPI phys via DT help alleviate this problem. In the DT case,
> > we'll look for a ULPI bus node underneath the device registering
> > the ULPI viewport (or the parent of that device to support
> > chipidea's device layout) and then match up the phy node
> > underneath that with the ULPI device that's created.
> >
> > The side benefit of this is that we can use standard properties
> > in the phy node like clks, regulators, gpios, etc. because we
> > don't have firmware like ACPI to turn these things on for us. And
> > we can use the DT phy binding to point our phy consumer to the
> > phy provider.
> >
> > The ULPI bus code supports native enumeration by reading the
> > vendor ID and product ID registers at device creation time, but
> > we can't be certain that those register reads will succeed if the
> > phy is not powered up. To avoid any problems with reading the ID
> > registers before the phy is powered we fallback to DT matching
> > when the ID reads fail.
> >
> > If the ULPI spec had some generic power sequencing for these
> > registers we could put that into the ULPI bus layer and power up
> > the device before reading the ID registers. Unfortunately this
> > doesn't exist and the power sequence is usually device specific.
> > By having the device matched up with DT we can avoid this
> > problem.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Cc: <devicetree@vger.kernel.org>
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
>
> Greg, is it ok I pick up this patch, and send it with chipidea
> changes together for 4.11-rc1 later?
No objection from me.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v5 5/5] i2c: mux: pca954x: Add irq-mask-enable to delay enabling irqs
From: Phil Reid @ 2017-01-19 7:48 UTC (permalink / raw)
To: Peter Rosin, wsa, robh+dt, mark.rutland, linux-i2c, devicetree
In-Reply-To: <312e32e9-a828-d692-895b-4042729e5417@axentia.se>
On 18/01/2017 20:19, Peter Rosin wrote:
> On 2017-01-17 09:00, Phil Reid wrote:
>> Unfortunately some hardware device will assert their irq line immediately
>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>> provide no method to mask irq line this provides a work around by keeping
>> the parent irq masked until enough device drivers have loaded to service
>> all pending interrupts.
>>
>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> device is registered irq are enabled and fire continuously as the second
>> device driver has not yet loaded. Setting this parameter to <1 1> will
>> delay the irq being enabled until both devices are ready.
>>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>> ---
>> drivers/i2c/muxes/i2c-mux-pca954x.c | 33 ++++++++++++++++++++++++++++++---
>> 1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index f55da88..012b2ef 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -76,6 +76,19 @@ struct chip_desc {
>> } muxtype;
>> };
>>
>> +/*
>> + * irq_mask_enable: Provides a mechanism to work around hardware that asserts
>> + * their irq immediately on power on. It allows the enabling of the irq to be
>> + * delayed until the corresponding bits in the the irq_mask are set thru
>> + * irq_unmask.
>> + * For example the ltc1760 assert its SMBALERT irq immediately on power on.
>> + * With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> + * device is registered irq are enabled and fire continuously as the second
>> + * device driver has not yet loaded. Setting this parameter to 0x3 while
>> + * delay the irq being enabled until both devices are ready.
>> + * This workaround will not work if two devices share an interrupt on the
>> + * same bus segment.
>
> It will also not work if something shares the interrupt with the pca954x mux,
> on the parent side of the mux, so to speak. Then that other driver may
> potentially enable the irq "behind the back" of the pca954x driver.
>
>> + */
>> struct pca954x {
>> const struct chip_desc *chip;
>>
>> @@ -84,7 +97,9 @@ struct pca954x {
>> struct i2c_client *client;
>>
>> struct irq_domain *irq;
>> + unsigned int irq_mask_enable;
>> unsigned int irq_mask;
>> + bool irq_enabled;
>> spinlock_t lock;
>> };
>>
>> @@ -266,8 +281,10 @@ static void pca954x_irq_mask(struct irq_data *idata)
>> spin_lock_irqsave(&data->lock, flags);
>>
>> data->irq_mask &= ~BIT(pos);
>> - if (!data->irq_mask)
>> + if (data->irq_enabled && !data->irq_mask) {
>> disable_irq(data->client->irq);
>> + data->irq_enabled = false;
>> + }
>
> When irq_mask_enable is non-zero, I think the parent irq should be masked
> when the first irq from the set in irq_mask_enable is masked. For symmetry.
>
> Like so (untested):
>
> if (data->irq_enabled) {
> if (!data->irq_mask ||
> (data->irq_mask & mask_enable) != mask_enable) {
> disable_irq(data->client->irq);
> data->irq_enabled = false;
> }
> }
Yeap this make sense.
>
> Hmm, this whole thing is fiddly and while it solves your problem it doesn't
> allow for solving the more general problem when there are "problematic"
> devices mixed with other devices. At least, I don't see it. And the
> limitations we are walking into with tracking number of enables etc suggests
> that we are attacking this at the wrong level. Maybe you should try to work
> around the hw limitations not in the pca954x driver, but in the irq core?
I'm looking at the option of getting the hardware changed to not route
the irq for my chips thru the i2c mux. Fortunately the hardware is going thru a
revision for some other changes. Messing with the irq core sounds dangerous
with my level of knowledge.
The other way I think I can tackle it after reading the datasheet for the ltc1760 is that
it'll deassert it's irq (smbalert) line when the host sends a ARA request on the bus segment.
There's a driver in the kernel for this already, but it's not DT enable and doesn't
handle multiple bus segments. I'll have a look at that as well.
Pretty sure it would need the mux to become an irq parent as per patch 1-3 of this series.
This would be so the system can figure out which segment to do the poll on.
But p4-5 could be dropped which is where we're stuck I think.
Looking at this approach it shouldn't matter if the ltc1760 driver has registered yet or not.
This approach possibly has a lot more generic appeal I think.
Thoughts on just submitting p1-3 for now while I figure out the SMB alert approach?
>
> I.e. have the irq core check, for each irq, for a property that specifies
> the depth at which each irq should be unmasked. This new property should
> probably be located in the interrupt-controller node? Then the code can
> unmask interrupts when the depth hits this mark, instead of always unmasking
> the interrupt when the depth changes from zero to one. You are then adding
> the workaround at a level where there is enough information to fix the
> more general problem. I think?
>
> But, once again, I'm no irq expert and would desperately like a second
> opinion on this stuff...
>
>>
>> spin_unlock_irqrestore(&data->lock, flags);
>> }
>> @@ -275,14 +292,18 @@ static void pca954x_irq_mask(struct irq_data *idata)
>> static void pca954x_irq_unmask(struct irq_data *idata)
>> {
>> struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> + unsigned int mask_enable = data->irq_mask_enable;
>> unsigned int pos = idata->hwirq;
>> unsigned long flags;
>>
>> spin_lock_irqsave(&data->lock, flags);
>>
>> - if (!data->irq_mask)
>> - enable_irq(data->client->irq);
>> data->irq_mask |= BIT(pos);
>> + if (!data->irq_enabled
>> + && (data->irq_mask & mask_enable) == mask_enable) {
>
> I think the coding standard says that the && should be at the end of the
> first line. Didn't checkpatch complain?
No it didn't complain. and I wasn't sure which way to do this.
So I had a look at some other code in the i2c folders (which is not definitive I know).
regexp "^\s*&&" showed some hits with this style.
Name Line Text Path
drivers\i2c\busses\i2c-ali1535.c 294 && (timeout++ < MAX_TIMEOUT));
drivers\i2c\busses\i2c-ali15x3.c 301 && (timeout++ < MAX_TIMEOUT));
drivers\i2c\busses\i2c-ismt.c 408 && (size != I2C_SMBUS_I2C_BLOCK_DATA))
drivers\i2c\busses\i2c-mv64xxx.c 258 && (drv_data->byte_posn != 0))) {
...
>
> Cheers,
> peda
>
>> + enable_irq(data->client->irq);
>> + data->irq_enabled = true;
>> + }
>>
>> spin_unlock_irqrestore(&data->lock, flags);
>> }
>> @@ -305,6 +326,7 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>> {
>> struct pca954x *data = i2c_mux_priv(muxc);
>> struct i2c_client *client = data->client;
>> + u32 irq_mask_enable[PCA954X_MAX_NCHANS] = { 0 };
>> int c, err, irq;
>>
>> if (!data->chip->has_irq || client->irq <= 0)
>> @@ -312,6 +334,9 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>>
>> spin_lock_init(&data->lock);
>>
>> + of_property_read_u32_array(client->dev.of_node, "nxp,irq-mask-enable",
>> + irq_mask_enable, data->chip->nchans);
>> +
>> data->irq = irq_domain_add_linear(client->dev.of_node,
>> data->chip->nchans,
>> &irq_domain_simple_ops, data);
>> @@ -319,6 +344,8 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>> return -ENODEV;
>>
>> for (c = 0; c < data->chip->nchans; c++) {
>> + data->irq_mask_enable |= irq_mask_enable[c] ? BIT(c) : 0;
>> + WARN_ON(irq_mask_enable[c] > 1);
>> irq = irq_create_mapping(data->irq, c);
>> irq_set_chip_data(irq, data);
>> irq_set_chip_and_handler(irq, &pca954x_irq_chip,
>>
>
>
>
--
Regards
Phil Reid
^ permalink raw reply
* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-19 7:53 UTC (permalink / raw)
To: Rob Herring
Cc: Thomas Petazzoni, Marek Vasut, Mark Rutland, devicetree,
Guochun Mao, Richard Weinberger, Russell King, linux-kernel,
linux-mtd, Matthias Brugger, linux-mediatek, Cyrille Pitchen,
Brian Norris, David Woodhouse, linux-arm-kernel
In-Reply-To: <20170118222010.ivc6jxpnrumemvdf@rob-hp-laptop>
On Wed, 18 Jan 2017 16:20:10 -0600
Rob Herring <robh@kernel.org> wrote:
> On Tue, Jan 17, 2017 at 02:36:50PM +1100, Thomas Petazzoni wrote:
> > Hello,
> >
> > (Side note: you guys should learn about stripping irrelevant parts of
> > an e-mail when replying!)
> >
> > On Mon, 16 Jan 2017 09:40:32 +0100, Boris Brezillon wrote:
> >
> > > > Well this is OK I guess, but then you can also use "mediatek,mt8173-nor"
> > > > as the oldest supported compatible and be done with it, no ? It looks a
> > > > bit crappy though, I admit that ...
> > >
> > > Let's stop bikeshedding and wait for DT maintainers feedback
> > > before taking a decision ;-).
> > >
> > > Rob, Mark, any opinion?
> >
>
> Sigh, is how to do compatibles really not yet understood?
Apparently not, and I fear this is not the last misunderstanding on my
side ;-).
>
> > I agree that a clarification would be good. There are really two
> > options:
> >
> > 1. Have two compatible strings in the DT, the one that matches the
> > exact SoC where the IP is found (first compatible string) and the
> > one that matches some other SoC where the same IP is found (second
> > compatible string). Originally, Linux only supports the second
> > compatible string in its device driver, but if it happens that a
> > difference is found between two IPs that we thought were the same,
> > we can add support for the first compatible string in the driver,
> > with a slightly different behavior.
>
> This. And no wildcards in the compatible string.
>
> > 2. Have a single compatible string in the DT, matching the exact SoC
> > where the IP is found. This involves adding immediately this
> > compatible string in the corresponding driver.
>
> I wouldn't object to this from a DT perspective as I have no clue
> generally if IP blocks are "the same" or not. Subsystem maintainers will
> object though.
>
> > I've not really been able to figure out which of the two options is the
> > most future-proof/appropriate.
>
> They are both future-proof. #2 has the disadvantage of requiring a
> kernel update for a new SoC.
>
> Rob
^ permalink raw reply
* Re: [PATCH v9 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2017-01-19 8:02 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: Wolfram Sang, Rob Herring, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Patrice Chotard, Russell King, linux-i2c,
devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <CAOAejn2LobTMfvsvmW8cCiToFHuM6n26s5QPLLXxLzb-WKkhQw@mail.gmail.com>
Hello Cedric,
On Wed, Jan 18, 2017 at 09:55:39PM +0100, M'boumba Cedric Madianga wrote:
> 2017-01-18 19:42 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > Hello Cedric,
> >
> > On Wed, Jan 18, 2017 at 04:21:17PM +0100, M'boumba Cedric Madianga wrote:
> >> >> + * In standard mode, the maximum allowed SCL rise time is 1000 ns.
> >> >> + * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
> >> >> + * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
> >> >> + * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
> >> >
> >> > * programmed with 0x9.
> >> > (1000 ns / 125 ns = 8)
> >> >
> >> >> + * So, for I2C standard mode TRISE = FREQ[5:0] + 1
> >> >> + *
> >> >> + * In fast mode, the maximum allowed SCL rise time is 300 ns.
> >> >> + * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
> >> >> + * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
> >> >> + * programmed with 03h.(300 ns / 125 ns = 2 + 1)
> >> >
> >> > as above s/03h/0x3/;
> >>
> >> ok
> >>
> >> > s/.(/. (/;
> >> ok
> >>
> >> > s/+ 1//;
> >> This formula is use to understand how we find the result 0x3
> >> So, 0x3 => 300 ns / 125ns = 2 + 1
> >
> > Yeah, I understood that, but writing 300 ns / 125ns = 2 + 1 is
> > irritating at best.
>
> Ok. I will write 0x3 (300 ns / 125 ns + 1) and 0x9 (1000 ns / 125 ns + 1)
>
> >> > [...]
> >> > If DUTY = 1: (to reach 400 kHz)
> >> >
> >> > Strange.
> >> >
> >> >> + val = DIV_ROUND_UP(i2c_dev->parent_rate, 400000 * 3);
> >> >
> >> > the manual reads:
> >> >
> >> > The minimum allowed value is 0x04, except in FAST DUTY mode
> >> > where the minimum allowed value is 0x01
> >> >
> >> > You don't check for that, right?
> >>
> >> As the minimum freq value is 6 Mhz in fast mode the minimum CCR is 5
> >> as described in the comment.
> >> So I don't need to check that again as it is already done by checking
> >> parent frequency.
> >
> > That would then go into a comment.
>
> Is it really needed ?
> Adding some comments to explain implementation choices or hardware
> way of working is clearly useful.
> But for this kind of thing, I am really surprised...
TL;DR: It's not needed, but it probably helps.
Consider someone sees a breakage in your driver in five years. By then
you either have other interests or at least forgot 95 % of the thoughts
you had when implementing the driver.
So when I see:
val = DIV_ROUND_UP(i2c_dev->parent_rate, 400000 * 3);
ccr |= STM32F4_I2C_CCR_CCR(val);
writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
after seeing that the bus freq is wrong the obvious thoughts are:
- Does this use the right algorithm?
- Does this calculation result in values that are usable by the
hardware?
That you thought about this today doesn't mean it's still right in five
years. During that time a new hardware variant is available with a
higher parent freq. Or there is a new errata available for the SoC.
So to help answer the questions above it helps if you add today the
formulas from the manual and a quick reason for why val fits into the
respective bits in the CCR register. That comment might be wrong until
then, too, but that only means you should make it easy to verify.
Something like:
/*
* Function bla_blub made sure that parent_rate is not higher
* than 23 * pi MHz. As a result val is at most 13.2 bits wide
* and so fits into the CCR bits.
*/
This gives you in five years time the opportunity to quickly check
bla_blub if this is still true, add a printk for parent_rate to check
this, or quickly identify the bug in the code or the mismatch to the
manual.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ 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