* Re: [PATCH v7 04/14] ARM: dts: Add xo_clock to sdhc nodes on qcom platforms
From: Ritesh Harjani @ 2016-11-15 5:10 UTC (permalink / raw)
To: Stephen Boyd, adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
andy.gross-QSEj5FYQhm4dnm+yROfE0A
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ, kdorfman-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
rnayak-sgV2jX0FEOL9JmXXK+q4OQ,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <20161114200115.GL5177-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 11/15/2016 1:31 AM, Stephen Boyd wrote:
> On 11/14, Ritesh Harjani wrote:
>> Add xo_clock to sdhc clock node on all qcom platforms.
>>
>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> arch/arm/boot/dts/qcom-apq8084.dtsi | 14 ++++++++------
>> arch/arm/boot/dts/qcom-msm8974.dtsi | 14 ++++++++------
>> arch/arm64/boot/dts/qcom/msm8916.dtsi | 10 ++++++----
>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +++++----
>> 4 files changed, 27 insertions(+), 20 deletions(-)
>>
>
> Is there an update to
> Documentation/devicetree/bindings/mmc/sdhci-msm.txt as well?
Sure, I will update.
>
--
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 v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Ritesh Harjani @ 2016-11-15 5:10 UTC (permalink / raw)
To: Stephen Boyd, adrian.hunter
Cc: ulf.hansson, linux-mmc, shawn.lin, andy.gross, devicetree,
linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
david.griego, stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <20161114193004.GI5177@codeaurora.org>
Hi Stephen/Adrian,
On 11/15/2016 1:07 AM, Stephen Boyd wrote:
> On 11/14, Ritesh Harjani wrote:
>> @@ -577,6 +578,90 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> return SDHCI_MSM_MIN_CLOCK;
>> }
>>
>> +/**
>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>> + *
>> + * Description:
>> + * Implement MSM version of sdhci_set_clock.
>> + * This is required since MSM controller does not
>> + * use internal divider and instead directly control
>> + * the GCC clock as per HW recommendation.
>> + **/
>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> + u16 clk;
>> + unsigned long timeout;
>> +
>> + /*
>> + * Keep actual_clock as zero -
>> + * - since there is no divider used so no need of having actual_clock.
>> + * - MSM controller uses SDCLK for data timeout calculation. If
>> + * actual_clock is zero, host->clock is taken for calculation.
>> + */
>> + host->mmc->actual_clock = 0;
>> +
>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>> +
>> + if (clock == 0)
>> + return;
>> +
>> + /*
>> + * MSM controller do not use clock divider.
>> + * Thus read SDHCI_CLOCK_CONTROL and only enable
>> + * clock with no divider value programmed.
>> + */
>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> +
>> + clk |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +
>> + /* Wait max 20 ms */
>> + timeout = 20;
>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> + & SDHCI_CLOCK_INT_STABLE)) {
>> + if (timeout == 0) {
>> + pr_err("%s: Internal clock never stabilised\n",
>> + mmc_hostname(host->mmc));
>> + return;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +
>> + clk |= SDHCI_CLOCK_CARD_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
> This is almost a copy/paste of sdhci_set_clock(). Can we make
> sdhci_set_clock() call a __sdhci_set_clock() function that takes
> unsigned int clock, and also a flag indicating if we want to set
> the internal clock divider or not? Then we can call
> __sdhci_set_clock() from sdhci_set_clock() with (clock, true) as
> arguments and (clock, false).
Adrian,
Could you please comment here ?
>
>> +}
>> +
>> +/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
>> +static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int rc;
>> +
>> + if (!clock) {
>> + msm_host->clk_rate = clock;
>> + goto out;
>> + }
>> +
>> + spin_unlock_irq(&host->lock);
>> + if (clock != msm_host->clk_rate) {
>
> Why do we need to check here? Can't we call clk_set_rate()
> Unconditionally?
Since it may so happen that above layers may call for ->set_clock
function with same requested clock more than once, hence we cache the
host->clock here.
Also, since requested clock (host->clock) can be say 400Mhz but the
actual pltfm supported clock would be say 384MHz.
>
>> + rc = clk_set_rate(msm_host->clk, clock);
>> + if (rc) {
>> + pr_err("%s: Failed to set clock at rate %u\n",
>> + mmc_hostname(host->mmc), clock);
>> + spin_lock_irq(&host->lock);
>> + goto out;
>
> Or replace the above two lines with goto err;
Ok, I will have another label out_lock instead of err.
>
>> + }
>> + msm_host->clk_rate = clock;
>> + pr_debug("%s: Setting clock at rate %lu\n",
>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>> + }
>
> And put an err label here.
will put the label here as out_lock;
>
>> + spin_lock_irq(&host->lock);
>> +out:
>> + __sdhci_msm_set_clock(host, clock);
>> +}
>> +
>> static const struct of_device_id sdhci_msm_dt_match[] = {
>> { .compatible = "qcom,sdhci-msm-v4" },
>> {},
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v7 13/14] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Ritesh Harjani @ 2016-11-15 4:24 UTC (permalink / raw)
To: Stephen Boyd
Cc: ulf.hansson, linux-mmc, adrian.hunter, shawn.lin, andy.gross,
devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
david.griego, stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <20161114195928.GK5177@codeaurora.org>
On 11/15/2016 1:29 AM, Stephen Boyd wrote:
> On 11/14, Ritesh Harjani wrote:
>> @@ -575,6 +729,15 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> +
>> + spin_unlock_irq(&host->lock);
>> + /* CDCLP533 HW calibration is only required for HS400 mode*/
>> + if (host->clock > CORE_FREQ_100MHZ &&
>> + msm_host->tuning_done && !msm_host->calibration_done &&
>> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>
> Drop useless parenthesis.
Ok, sure.
>
>> + if (!sdhci_msm_cdclp533_calibration(host))
>> + msm_host->calibration_done = true;
>> + spin_lock_irq(&host->lock);
>> }
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v7 14/14] sdhci: sdhci-msm: update dll configuration
From: Ritesh Harjani @ 2016-11-15 4:23 UTC (permalink / raw)
To: Stephen Boyd
Cc: ulf.hansson, linux-mmc, adrian.hunter, shawn.lin, andy.gross,
devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
david.griego, stummala, venkatg, rnayak, pramod.gurav,
Krishna Konda
In-Reply-To: <20161114195749.GJ5177@codeaurora.org>
On 11/15/2016 1:27 AM, Stephen Boyd wrote:
> On 11/14, Ritesh Harjani wrote:
>> @@ -903,7 +998,33 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> config |= CORE_HC_SELECT_IN_EN;
>> writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> }
>> + if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
>> + /*
>> + * Poll on DLL_LOCK or DDR_DLL_LOCK bits in
>> + * CORE_DLL_STATUS to be set. This should get set
>> + * within 15 us at 200 MHz.
>> + */
>> + rc = readl_relaxed_poll_timeout(host->ioaddr +
>> + CORE_DLL_STATUS,
>> + dll_lock,
>> + (dll_lock &
>> + (CORE_DLL_LOCK |
>> + CORE_DDR_DLL_LOCK)), 10,
>> + 1000);
>> + if (rc == -ETIMEDOUT)
>> + pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
>> + mmc_hostname(host->mmc), dll_lock);
>> + }
>> } else {
>> + if (!msm_host->use_cdclp533) {
>> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
>
> These types of comments are totally useless. The code says
> exactly what is being done, and the comment is actually wrong in
> this case. Please remove all these "set/clear bit X in register
> Y" comments.
Ok, done.
>
>> + config = readl_relaxed(host->ioaddr +
>> + CORE_VENDOR_SPEC3);
>> + config &= ~CORE_PWRSAVE_DLL;
>> + writel_relaxed(config, host->ioaddr +
>> + CORE_VENDOR_SPEC3);
>> + }
>> +
>> /* Select the default clock (free running MCLK) */
>> config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> config &= ~CORE_HC_MCLK_SEL_MASK;
>> @@ -1100,6 +1221,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>> msm_host->use_14lpp_dll_reset = true;
>>
>> /*
>> + * SDCC 5 controller with major version 1, minor version 0x34 and later
>> + * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
>> + */
>> + if ((core_major == 1) && (core_minor < 0x34))
>
> Drop useless parenthesis please.
Done.
>
>> + msm_host->use_cdclp533 = true;
>> +
>> + /*
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v4 2/2] phy: rockchip-inno-usb2: correct 480MHz output clock stable time
From: William Wu @ 2016-11-15 3:54 UTC (permalink / raw)
To: kishon-l0cyMroinI0, heiko-4mtYJXux2i+zQB+pC5nmwQ
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
frank.wang-TNX95d0MmH7DzftRWevZcw,
huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA,
wulf-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1479182047-3399-1-git-send-email-wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
We found that the system crashed due to 480MHz output clock of
USB2 PHY was unstable after clock had been enabled by gpu module.
Theoretically, 1 millisecond is a critical value for 480MHz
output clock stable time, so we try to change the delay time
to 1.2 millisecond to avoid this issue.
And the commit ed907fb1d7c3 ("phy: rockchip-inno-usb2: correct
clk_ops callback") used prepare callbacks instead of enable
callbacks to support gate a clk if the operation may sleep. So
we can switch from delay to sleep functions.
Also fix a spelling error from "waitting" to "waiting".
Signed-off-by: William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v4:
- add Reviewed-by and fix a spelling error
Changes in v3:
- None
Changes in v2:
- None
drivers/phy/phy-rockchip-inno-usb2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
index 365e077..5d922fc 100644
--- a/drivers/phy/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/phy-rockchip-inno-usb2.c
@@ -165,8 +165,8 @@ static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
if (ret)
return ret;
- /* waitting for the clk become stable */
- mdelay(1);
+ /* waiting for the clk become stable */
+ usleep_range(1200, 1300);
}
return 0;
--
2.0.0
--
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 v4 1/2] phy: rockchip-inno-usb2: correct clk_ops callback
From: William Wu @ 2016-11-15 3:54 UTC (permalink / raw)
To: kishon, heiko
Cc: linux-kernel, linux-arm-kernel, linux-rockchip, devicetree,
robh+dt, frank.wang, huangtao, dianders, briannorris, groeck,
wulf
In-Reply-To: <1479182047-3399-1-git-send-email-wulf@rock-chips.com>
Since we needs to delay ~1ms to wait for 480MHz output clock
of USB2 PHY to become stable after turn on it, the delay time
is pretty long for something that's supposed to be "atomic"
like a clk_enable(). Consider that clk_enable() will disable
interrupt and that a 1ms interrupt latency is not sensible.
The 480MHz output clock should be handled in prepare callbacks
which support gate a clk if the operation may sleep.
Signed-off-by: William Wu <wulf@rock-chips.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- add Reviewed-by
Changes in v3:
- None
Changes in v2:
- None
drivers/phy/phy-rockchip-inno-usb2.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
index ac20310..365e077 100644
--- a/drivers/phy/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/phy-rockchip-inno-usb2.c
@@ -153,7 +153,7 @@ static inline bool property_enabled(struct rockchip_usb2phy *rphy,
return tmp == reg->enable;
}
-static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw)
+static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
{
struct rockchip_usb2phy *rphy =
container_of(hw, struct rockchip_usb2phy, clk480m_hw);
@@ -172,7 +172,7 @@ static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw)
return 0;
}
-static void rockchip_usb2phy_clk480m_disable(struct clk_hw *hw)
+static void rockchip_usb2phy_clk480m_unprepare(struct clk_hw *hw)
{
struct rockchip_usb2phy *rphy =
container_of(hw, struct rockchip_usb2phy, clk480m_hw);
@@ -181,7 +181,7 @@ static void rockchip_usb2phy_clk480m_disable(struct clk_hw *hw)
property_enable(rphy, &rphy->phy_cfg->clkout_ctl, false);
}
-static int rockchip_usb2phy_clk480m_enabled(struct clk_hw *hw)
+static int rockchip_usb2phy_clk480m_prepared(struct clk_hw *hw)
{
struct rockchip_usb2phy *rphy =
container_of(hw, struct rockchip_usb2phy, clk480m_hw);
@@ -197,9 +197,9 @@ rockchip_usb2phy_clk480m_recalc_rate(struct clk_hw *hw,
}
static const struct clk_ops rockchip_usb2phy_clkout_ops = {
- .enable = rockchip_usb2phy_clk480m_enable,
- .disable = rockchip_usb2phy_clk480m_disable,
- .is_enabled = rockchip_usb2phy_clk480m_enabled,
+ .prepare = rockchip_usb2phy_clk480m_prepare,
+ .unprepare = rockchip_usb2phy_clk480m_unprepare,
+ .is_prepared = rockchip_usb2phy_clk480m_prepared,
.recalc_rate = rockchip_usb2phy_clk480m_recalc_rate,
};
--
2.0.0
^ permalink raw reply related
* [PATCH v4 0/2] phy: rockchip-inno-usb2: correct 480MHz clk_ops callbacks and stable time
From: William Wu @ 2016-11-15 3:54 UTC (permalink / raw)
To: kishon-l0cyMroinI0, heiko-4mtYJXux2i+zQB+pC5nmwQ
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
frank.wang-TNX95d0MmH7DzftRWevZcw,
huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA,
wulf-TNX95d0MmH7DzftRWevZcw
This series try to correct the 480MHz output clock of USB2 PHY
clk_ops callback and fix the delay time. It aims to make the
480MHz clock gate more sensible and stable.
Tested on rk3366/rk3399 EVB board.
William Wu (2):
phy: rockchip-inno-usb2: correct clk_ops callback
phy: rockchip-inno-usb2: correct 480MHz output clock stable time
drivers/phy/phy-rockchip-inno-usb2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Archit Taneja @ 2016-11-15 3:35 UTC (permalink / raw)
To: Rob Herring
Cc: Laurent Pinchart, Andy Gross, linux-arm-msm,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+KMPoyWvh1U3OKL6pX8D4QByqVFgcMks8myDfZEYeFCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/10/2016 07:04 AM, Rob Herring wrote:
> On Fri, Oct 14, 2016 at 12:40 AM, Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Hi Rob,
>>
>>
>> On 09/26/2016 01:42 PM, Laurent Pinchart wrote:
>>>
>>> Hi Archit,
>>>
>>> On Monday 26 Sep 2016 12:10:11 Archit Taneja wrote:
>>>>
>>>> On 9/25/2016 10:58 PM, Laurent Pinchart wrote:
>>>>>
>>>>> On Friday 23 Sep 2016 14:50:27 Archit Taneja wrote:
>>>>>>
>>>>>> Add the regulator supply properties needed by ADV7511 and ADV7533. The
>>>>>> regulator names are named after the pin names, except for DVDD_3V on
>>>>>> ADV7511, which is left v3p3 to have the same name as it's on ADV7533.
>>>>>>
>>>>>> ADV7511 has a BGVDD pin for which the voltage to be configured isn't
>>>>>> clear. This needs to be updated in the document later.
>>>>>>
>>>>>> The regulators are specified as optional properties since there can
>>>>>> be boards which have a fixed supply directly routed to the pins, and
>>>>>> these may not be modelled as regulator supplies.
>>>>>>
>>>>>> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>>> Signed-off-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>>>> ---
>>>>>> v2:
>>>>>> - Specify regulator supplies for ADV7511 too.
>>>>>> - Have separate supply entries for AVDD, DVDD, PVDD, A2VDD pins.
>>>>>> - Add additional BGVDD supply for ADV7511.
>>>>>>
>>>>>> .../devicetree/bindings/display/bridge/adi,adv7511.txt | 14
>>>>>> +++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> index
>>>>>> 6532a59..66e6bae 100644
>>>>>> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
>>>>>>
>>>>>> @@ -56,6 +56,20 @@ Optional properties:
>>>>>> - adi,disable-timing-generator: Only for ADV7533. Disables the
>>>>>> internal
>>>>>>
>>>>>> timing generator. The chip will rely on the sync signals in the DSI
>>>>>> data
>>>>>> lanes, rather than generate its own timings for HDMI output.
>>>>>> +- avdd-supply: A 1.8V supply that powers up the AVDD pin on the chip.
>>>>>> +- dvdd-supply: A 1.8V supply that powers up the DVDD pin on the chip.
>>>>>> +- pvdd-supply: A 1.8V supply that powers up the PVDD pin on the chip.
>>>>>> +- v3p3-supply: A 3.3V supply that powers up the pin called DVDD_3V on
>>>>>> + ADV7511 and V3P3 on ADV7533.
>>>>>> +
>>>>>> +ADV7511 specific supplies:
>>>>>> +- bgvdd-supply: Powers the BGVDD pin. The voltage isn't clear from
>>>>>> + available datasheets.
>>>>>
>>>>>
>>>>> It's a 1.8V supply. The ADV7511 hardware user's guide recommends tying
>>>>> it
>>>>> to the PVDD supply, so I'm not sure we need a separate supply in DT.
>>>>
>>>>
>>>> Thanks for providing the info. I'll update it in the doc.
>>>>
>>>>> Similarly, the user's guide recommends supplying the AVDD, DVDD and PVDD
>>>>> pins with a single 1.8V LDO and three independent filters. We could thus
>>>>> possibly combine them into a single 1.8V supply.
>>>>
>>>>
>>>> I'd originally specified only one AVDD supply for all the 1.8 V
>>>> supplies, but Rob had recommended using separate supplies for each pin.
>>>>
>>>> I guess it doesn't do much harm specifying separate supplies, a
>>>> board can just provide avdd-supply, and the rest would be assumed dummy.
>>>
>>>
>>> DT should use the same regulator phandle for all power supplies in that
>>> case,
>>> not specify a single one.
>>>
>>>> It could also help describe boards which may not have followed the
>>>> user guide's recommendation, but I agree that's probably a rare
>>>> scenario.
>>>>
>>>> Is it okay if we stick with multiple supplies as it is in this version?
>>>
>>>
>>> It seems overkill to me but I can live with that. I'd remove the bgvdd
>>> supply
>>> though.
>>>
>>> Rob, what's your opinion on this ? Is there a specific reason why you
>>> prefer
>>> splitting the 1.8V supply in one supply per pin, when the hardware user's
>>> guide recommend powering them from the same LDO ?
>>
>>
>> Could you suggest us which way to go?
>
> If they are really intended to be the same supply, then combining them
> is fine with me.
Thanks, I'll change my patchset accordingly.
Archit
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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 V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Viresh Kumar @ 2016-11-15 3:31 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rob Herring, Mark Brown, Rafael Wysocki, nm, Viresh Kumar,
linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, d-gerlach,
devicetree
In-Reply-To: <20161115021302.GR5177@codeaurora.org>
First of all, thanks to all of you for commenting here. Please
continue doing so as I want to finish this stuff quickly, it has
already killed a lot of time :)
On 14-11-16, 18:13, Stephen Boyd wrote:
> On 11/14, Rob Herring wrote:
> > On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> > > On 10-11-16, 14:51, Stephen Boyd wrote:
> > > >
> > > > No. The supply names (and also clock names/index) should be left
> > > > up to the consumer of the OPP table. We don't want to encode any
> > > > sort of details like this between the OPP table and the consumer
> > > > of it in DT because then it seriously couples the OPP table to
> > > > the consumer device. "The binding" in this case that needs to be
> > > > updated is the consumer binding, to indicate that it correlated
> > > > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > > > voltages.
> > >
> > > Are you saying that we shall have a property like this then?
> > >
> > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > > index ee91cbdd95ee..733946df2fb8 100644
> > > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > > @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
> > > compatible = "arm,cortex-a7";
> > > ...
> > >
> > > - cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > > + vcc0-supply = <&cpu_supply0>;
> > > + vcc1-supply = <&cpu_supply1>;
> > > + vcc2-supply = <&cpu_supply2>;
> > > + opp-supply-names = "vcc0", "vcc1", "vcc2";
> >
> > Uh, no. You already have the names in the *-supply properties. Yes, they
> > are a PIA to retrieve compared to a *-names property, but that is the
> > nature of this style of binding.
Its not just PIA, but impossible AFAICT.
There are two important pieces of information we need for multiple
regulator support:
- Which regulator in the consumer node corresponds to which entry in
the OPP table. As Mark mentioned earlier, DT should be able to get
us this.
- The order in which the supplies need to be programmed. We have all
agreed to do this in code instead of inferring it from DT and this
patch series already does that.
I want to solve the first problem here and I don't see how it can be
solved using such entries:
cpus {
cpu@0 {
compatible = "arm,cortex-a7";
...
vcc0-supply = <&cpu_supply0>;
vcc1-supply = <&cpu_supply1>;
vcc2-supply = <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-shared;
opp@1000000000 {
opp-hz = /bits/ 64 <1000000000>;
opp-microvolt = <970000>, /* Supply 0 */
<960000>, /* Supply 1 */
<960000>; /* Supply 2 */
};
};
The code can't figure out which of vcc0, vcc1, vcc2 is added first in
the CPU node and so we need to get the order somehow. A separate
binding as I mentioned earlier is a probably (ugly) solution.
> I think the problem is that Viresh wants the binding to be "self
> describing" so that the OPP can be used without a driver knowing
> that a supply corresponds to a particular column in the voltage
> table.
Right, and that's what Mark suggested as well.
> I don't understand that though. Can't we set the supply
> names from C code somewhere based on the consumer of the OPPs?
That's what this patch series is doing right now.
So, are you saying that the way this patchset does it is fine with you
?
--
viresh
^ permalink raw reply
* Re: [PATCH v3 2/2] phy: rockchip-inno-usb2: correct 480MHz output clock stable time
From: wlf @ 2016-11-15 3:23 UTC (permalink / raw)
To: Doug Anderson
Cc: Kishon Vijay Abraham I, Heiko Stübner,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
open list:ARM/Rockchip SoC...,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Frank Wang, 黄涛, Brian Norris, Guenter Roeck
In-Reply-To: <CAD=FV=VJsuiXV3NS0uroF2OWJEDn5eiCpX-jP3BOYgLZ8gy6Hw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
在 2016年11月15日 02:17, Doug Anderson 写道:
> William,
>
> On Mon, Nov 14, 2016 at 1:27 AM, William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> We found that the system crashed due to 480MHz output clock of
>> USB2 PHY was unstable after clock had been enabled by gpu module.
>>
>> Theoretically, 1 millisecond is a critical value for 480MHz
>> output clock stable time, so we try to change the delay time
>> to 1.2 millisecond to avoid this issue.
>>
>> And the commit ed907fb1d7c3 ("phy: rockchip-inno-usb2: correct
>> clk_ops callback") used prepare callbacks instead of enable
>> callbacks to support gate a clk if the operation may sleep. So
>> we can switch from delay to sleep functions.
>>
>> Signed-off-by: William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>> Changes in v3:
>> - fix kbuild test error: too few arguments to function 'usleep_range'
>>
>> Changes in v2:
>> - use usleep_range() function instead of mdelay()
>>
>> drivers/phy/phy-rockchip-inno-usb2.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
>> index 365e077..0e52b25 100644
>> --- a/drivers/phy/phy-rockchip-inno-usb2.c
>> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
>> @@ -166,7 +166,7 @@ static int rockchip_usb2phy_clk480m_prepare(struct clk_hw *hw)
>> return ret;
>>
>> /* waitting for the clk become stable */
>> - mdelay(1);
>> + usleep_range(1200, 1300);
> Sight nit that you could also fix the spelling from "waitting" to "waiting".
>
> ...but that's pre-existing, so:
>
> Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Thanks! I'll add Reviewed-by and fix the spelling issue.
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] phy: rockchip-inno-usb2: correct clk_ops callback
From: wlf @ 2016-11-15 3:22 UTC (permalink / raw)
To: Doug Anderson
Cc: Kishon Vijay Abraham I, Heiko Stübner,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
open list:ARM/Rockchip SoC...,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Frank Wang, 黄涛, Brian Norris, Guenter Roeck
In-Reply-To: <CAD=FV=W36tdw3LqPq3tp58U4trYX6n=qaotMTsP6tm0QzZrRWQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Doug,
在 2016年11月15日 02:15, Doug Anderson 写道:
> William
>
> On Sun, Nov 13, 2016 at 11:01 PM, William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
>> Since we needs to delay ~1ms to wait for 480MHz output clock
>> of USB2 PHY to become stable after turn on it, the delay time
>> is pretty long for something that's supposed to be "atomic"
>> like a clk_enable(). Consider that clk_enable() will disable
>> interrupt and that a 1ms interrupt latency is not sensible.
>>
>> The 480MHz output clock should be handled in prepare callbacks
>> which support gate a clk if the operation may sleep.
>>
>> Signed-off-by: William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>> drivers/phy/phy-rockchip-inno-usb2.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
> Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Thanks! I'll add Reviewed-by.
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] binding: irqchip: mtk-cirq: Add binding document
From: Youlin Pei @ 2016-11-15 3:09 UTC (permalink / raw)
To: Rob Herring
Cc: Marc Zyngier, Matthias Brugger, Thomas Gleixner, Jason Cooper,
Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20161109182613.7tvzewua6ewxdvjc@rob-hp-laptop>
On Wed, 2016-11-09 at 12:26 -0600, Rob Herring wrote:
> On Tue, Nov 01, 2016 at 07:52:00PM +0800, Youlin Pei wrote:
> > This commit adds the device tree binding document for
> > the mediatek cirq.
> >
> > Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> > .../interrupt-controller/mediatek,cirq.txt | 30 ++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> > new file mode 100644
> > index 0000000..84e8123
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> > @@ -0,0 +1,30 @@
> > +* Mediatek 27xx cirq
> > +
> > +In Mediatek SOCs, the CIRQ is a low power interrupt controller designed to
> > +works outside MCUSYS which comprises with Cortex-Ax cores,CCI and GIC.
>
> s/works/work/
will fix in next version.
>
> > +The external interrupts (outside MCUSYS) will feed through CIRQ and connect
> > +to GIC in MCUSYS. When CIRQ is enabled, it will record the edge-sensitive
> > +interrupts and generated a pulse signal to parent interrupt controller when
>
> s/generated/generate/
will fix in next version.
>
> > +flush command is executed. With CIRQ, MCUSYS can be completely turned off
> > +to improve the system power consumption without losing interrupts.
> > +
> > +Required properties:
> > +- compatible: should be: "mediatek,mtk-cirq".
>
> This should be SoC specific. This is fine as a fallback if the same
> block is in many SoCs, but mediatek and mtk is a bit redundant.
>
In next version, i will improve as following:
- compatible: Should be one of
- "mediatek,mt2701-cirq" for mt2701 CIRQ
- "mediatek,mt8173-cirq" for mt8173 CIRQ
- "mediatek,mt8135-cirq" for mt8135 CIRQ
and "mediatek,cirq" as a fallback.
> > +- interrupt-controller : Identifies the node as an interrupt controller.
> > +- #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
> > +- interrupt-parent: phandle of irq parent for cirq. The parent must
> > + use the same interrupt-cells format as GIC.
> > +- reg: Physical base address of the cirq registers and length of memory
> > + mapped region.
> > +- mediatek,ext-irq-start: Identifies external irq start number in different
> > + SOCs.
>
> Wouldn't this always be 32 if the GIC is the parent? If 32 is the common
> case, then use the SoC compatible to determine this value.
>
ext-irq-start is not always 32. In different Socs, this value is
different.
Thanks a lot!
> > +
> > +Example:
> > + cirq: interrupt-controller@10204000 {
> > + compatible = "mediatek,mtk-cirq";
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + interrupt-parent = <&sysirq>;
> > + reg = <0 0x10204000 0 0x4000>;
> > + mediatek,ext-irq-start = <32>;
> > + };
> > --
> > 1.7.9.5
> >
--
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: Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply
From: Chen-Yu Tsai @ 2016-11-15 2:59 UTC (permalink / raw)
To: David Airlie
Cc: Chen-Yu Tsai, Rob Herring, Maxime Ripard, Mark Rutland, dri-devel,
linux-arm-kernel, linux-kernel, devicetree, linux-sunxi
In-Reply-To: <CAGb2v64tTacSAG1vvO28h8MN9JC9w+OzSrP+j2eTzqAiQKgaSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>>> ---
>>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Any comments on this patch from the DRM people?
ChenYu
>>
>> One code comment below...
>>
>>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 35 ++++++++++++++++++++++
>>> 2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>> - Video port 0 for RGB input
>>> - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>> Example
>>> -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>> #include <linux/module.h>
>>> #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>> struct drm_connector connector;
>>>
>>> struct i2c_adapter *ddc;
>>> + struct regulator *vdd;
>>> };
>>>
>>> static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>> return 0;
>>> }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> + int ret;
>>> +
>>> + if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>> return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> + ret = regulator_enable(vga->vdd);
>>> +
>>> + if (ret) {
>>> + DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> + return;
>>> + }
>>> + }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> + if (!IS_ERR(vga->vdd))
>>> + regulator_disable(vga->vdd);
>>> +}
>>> +
>>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>> .attach = dumb_vga_attach,
>>> + .enable = dumb_vga_enable,
>>> + .disable = dumb_vga_disable,
>>> };
>>>
>>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>> return -ENOMEM;
>>> platform_set_drvdata(pdev, vga);
>>>
>>> + vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> + if (IS_ERR(vga->vdd)) {
>>> + ret = PTR_ERR(vga->vdd);
>>> + if (ret == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> + dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> + }
>>> +
>>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>> if (IS_ERR(vga->ddc)) {
>>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> 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.
^ permalink raw reply
* Re: [PATCH v2 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Youlin Pei @ 2016-11-15 2:49 UTC (permalink / raw)
To: Marc Zyngier
Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1f8f7b9f-7788-18c4-19c5-3c9a7623c188-5wv7dgnIgG8@public.gmane.org>
On Thu, 2016-11-10 at 18:24 +0000, Marc Zyngier wrote:
> On 08/11/16 02:57, Youlin Pei wrote:
> > On Fri, 2016-11-04 at 22:21 +0000, Marc Zyngier wrote:
> >> On Fri, Nov 04 2016 at 04:42:57 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>> On Tue, 2016-11-01 at 20:49 +0000, Marc Zyngier wrote:
> >>>> On Tue, Nov 01 2016 at 11:52:01 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>>>> In Mediatek SOCs, the CIRQ is a low power interrupt controller
> >>>>> designed to works outside MCUSYS which comprises with Cortex-Ax
> >>>>> cores,CCI and GIC.
> >>>>>
> >>>>> The CIRQ controller is integrated in between MCUSYS( include
> >>>>> Cortex-Ax, CCI and GIC ) and interrupt sources as the second
> >>>>> level interrupt controller. The external interrupts which outside
> >>>>> MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
> >>>>> all edge trigger interupts. When an edge interrupt is triggered,
> >>>>> CIRQ can record the status and generate a pulse signal to GIC when
> >>>>> flush command executed.
> >>>>>
> >>>>> When system enters sleep mode, MCUSYS will be turned off to improve
> >>>>> power consumption, also GIC is power down. The edge trigger interrupts
> >>>>> will be lost in this scenario without CIRQ.
> >>>>>
> >>>>> This commit provides the CIRQ irqchip implement.
> >>>>>
> >>>>> Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>> ---
> >>>>> drivers/irqchip/Makefile | 2 +-
> >>>>> drivers/irqchip/irq-mtk-cirq.c | 262 ++++++++++++++++++++++++++++++++++++++++
> >>>>> 2 files changed, 263 insertions(+), 1 deletion(-)
> >>>>> create mode 100644 drivers/irqchip/irq-mtk-cirq.c
> >>>>>
> >>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >>>>> index e4dbfc8..8f33580 100644
> >>>>> --- a/drivers/irqchip/Makefile
> >>>>> +++ b/drivers/irqchip/Makefile
> >>>>> @@ -60,7 +60,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ) += irq-bcm7120-l2.o
> >>>>> obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o
> >>>>> obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
> >>>>> obj-$(CONFIG_MIPS_GIC) += irq-mips-gic.o
> >>>>> -obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o
> >>>>> +obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o irq-mtk-cirq.o
> >>>>> obj-$(CONFIG_ARCH_DIGICOLOR) += irq-digicolor.o
> >>>>> obj-$(CONFIG_RENESAS_H8300H_INTC) += irq-renesas-h8300h.o
> >>>>> obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o
> >>>>> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> >>>>> new file mode 100644
> >>>>> index 0000000..fc43ef3
> >>>>> --- /dev/null
> >>>>> +++ b/drivers/irqchip/irq-mtk-cirq.c
> >>>>> @@ -0,0 +1,262 @@
> >>>>> +/*
> >>>>> + * Copyright (c) 2016 MediaTek Inc.
> >>>>> + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@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 version 2 as
> >>>>> + * published by the Free Software Foundation.
> >>>>> + *
> >>>>> + * This program is distributed in the hope that it will be useful,
> >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>>>> + * GNU General Public License for more details.
> >>>>> + */
> >>>>> +
> >>>>> +#include <linux/irq.h>
> >>>>> +#include <linux/irqchip.h>
> >>>>> +#include <linux/irqdomain.h>
> >>>>> +#include <linux/of.h>
> >>>>> +#include <linux/of_irq.h>
> >>>>> +#include <linux/of_address.h>
> >>>>> +#include <linux/io.h>
> >>>>> +#include <linux/slab.h>
> >>>>> +#include <linux/syscore_ops.h>
> >>>>> +
> >>>>> +#define CIRQ_ACK 0x40
> >>>>> +#define CIRQ_MASK_SET 0xc0
> >>>>> +#define CIRQ_MASK_CLR 0x100
> >>>>> +#define CIRQ_SENS_SET 0x180
> >>>>> +#define CIRQ_SENS_CLR 0x1c0
> >>>>> +#define CIRQ_POL_SET 0x240
> >>>>> +#define CIRQ_POL_CLR 0x280
> >>>>> +#define CIRQ_CONTROL 0x300
> >>>>> +
> >>>>> +#define CIRQ_EN 0x1
> >>>>> +#define CIRQ_EDGE 0x2
> >>>>> +#define CIRQ_FLUSH 0x4
> >>>>> +
> >>>>> +#define CIRQ_IRQ_NUM 0x200
> >>>>> +
> >>>>> +struct mtk_cirq_chip_data {
> >>>>> + void __iomem *base;
> >>>>> + unsigned int ext_irq_start;
> >>>>> +};
> >>>>> +
> >>>>> +static struct mtk_cirq_chip_data *cirq_data;
> >>>>
> >>>> Are you guaranteed that you'll only ever have a single CIRQ in any
> >>>> system?
> >>>
> >>> In Mediatek's SOC, only hace a single CIRQ.
> >>>
> >>>>
> >>>>> +
> >>>>> +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
> >>>>> +{
> >>>>> + struct mtk_cirq_chip_data *chip_data = data->chip_data;
> >>>>> + unsigned int cirq_num = data->hwirq;
> >>>>> + u32 mask = 1 << (cirq_num % 32);
> >>>>> +
> >>>>> + writel(mask, chip_data->base + offset + (cirq_num / 32) * 4);
> >>>>
> >>>> Why can't you use the relaxed accessors?
> >>>
> >>> It seems that i use wrong function, i will change the writel to
> >>> writel_relaxed in next ve
> >>>
> >>>>
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_mask(struct irq_data *data)
> >>>>> +{
> >>>>> + mtk_cirq_write_mask(data, CIRQ_MASK_SET);
> >>>>> + irq_chip_mask_parent(data);
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_unmask(struct irq_data *data)
> >>>>> +{
> >>>>> + mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
> >>>>> + irq_chip_unmask_parent(data);
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_eoi(struct irq_data *data)
> >>>>> +{
> >>>>> + mtk_cirq_write_mask(data, CIRQ_ACK);
> >>>>
> >>>> EOI and ACK have very different semantics. What is this write actually
> >>>> doing? Also, you're now doing an additional MMIO write on each interrupt
> >>>> EOI, doubling its cost. Do you really need to do actually signal the HW
> >>>> that we've EOIed an interrupt? I would have hoped that you'd be able to
> >>>> put it in "bypass" mode as long as you're not suspending...
> >>>>
> >>>
> >>> When external interrupt happened, CIRQ status register record the status
> >>> even CIRQ is not enabled. when execute the flush command, CIRQ will
> >>> resend the signals according to the status. So if don't clear the
> >>> status, CIRQ will resend the wrong signals. the ACK write operation will
> >>> clear the status.
> >>
> >> But at this time, we haven't suspended yet, and there is nothing to
> >> replay. Also, you only enable the edge capture when suspending. So what
> >> are you ACKing here? Can't you simply clear everything right when
> >> suspending and not do it at all on the fast path?
> >
> > I had planned to ACK the status in cirq suspend callback, but
> > encountered a problem.
> > following is a piece of code from
> > http://lxr.free-electrons.com/source/kernel/power/suspend.c#L361
> >
> > arch_suspend_disable_irqs(); ---step1
> > BUG_ON(!irqs_disabled());
> >
> > error = syscore_suspend();
> > |
> > ---cirq suspend(); ---step2
> >
> > if ack the status in cirq suspend, the interrupts will be lost which
> > happened during step1 to step2.
> >
> > So would you mind give me some suggestions about this?
> > Thanks a lot!
>
> Right. So maybe there is a way:
> - On suspend you can iterate over all the cirq interrupts that have been
> recorded
> - For each of those, you inspect if it is pending at the GIC level
> (using the irq_get_irqchip_state helper)
> - if not pending, you Ack it, because it cannot be delivered anymore
> - If it is pending, then you know it happened between step 1 and step 2.
> - You then have the choice of either going into suspend and waking up
> immediately, or failing the suspend.
>
> Thoughts?
Will use this solution in next version.
Thanks a lot!
>
> M.
--
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 2/3] Revert "Documentation: devicetree: dwc2: Add host DMA binding"
From: John Youn @ 2016-11-15 2:29 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
This reverts commit 751089ecaab0 ("Documentation: devicetree: dwc2: Add
host DMA binding").
Remove this binding as it is not needed by any hardware.
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 389bb13..ad8f7ff 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,7 +25,6 @@ Optional properties:
Refer to phy/phy-bindings.txt for generic phy consumer properties
- dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
-- snps,host-dma-disable: disable host DMA mode.
- g-rx-fifo-size: size of rx fifo size in gadget mode.
- g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
--
2.10.0
--
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 1/3] Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
From: John Youn @ 2016-11-15 2:29 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
This reverts commit 9acc1ee2b723 ("usb: dwc2: Add bindings to disable
gadget DMA modes").
Don't add bindings and don't read them in. These are not yet needed by
any hardware.
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 2 --
drivers/usb/dwc2/params.c | 9 ++-------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..389bb13 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,8 +26,6 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
- dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
- snps,host-dma-disable: disable host DMA mode.
-- snps,gadget-dma-disable: disable gadget DMA mode.
-- snps,gadget-dma-desc-disable: disable gadget DMA descriptor mode.
- g-rx-fifo-size: size of rx fifo size in gadget mode.
- g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..2f18a7b 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1091,22 +1091,17 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
struct dwc2_hw_params *hw = &hsotg->hw_params;
struct dwc2_core_params *p = &hsotg->params;
bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
- bool disable;
/* Buffer DMA */
- disable = device_property_read_bool(hsotg->dev,
- "snps,gadget-dma-disable");
dwc2_set_param_bool(hsotg, &p->g_dma,
false, "gadget-dma",
- !disable, false,
+ true, false,
dma_capable);
/* DMA Descriptor */
- disable = device_property_read_bool(hsotg->dev,
- "snps,gadget-dma-desc-disable");
dwc2_set_param_bool(hsotg, &p->g_dma_desc, false,
"gadget-dma-desc",
- p->g_dma && !disable, false,
+ p->g_dma, false,
!!hw->dma_desc_enable);
}
--
2.10.0
--
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 1/3] Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
From: John Youn @ 2016-11-15 2:29 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
This reverts commit 9acc1ee2b723 ("usb: dwc2: Add bindings to disable
gadget DMA modes").
Don't add bindings and don't read them in. These are not yet needed by
any hardware.
Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
Documentation/devicetree/bindings/usb/dwc2.txt | 2 --
drivers/usb/dwc2/params.c | 9 ++-------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..389bb13 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,8 +26,6 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
- dr_mode: shall be one of "host", "peripheral" and "otg"
Refer to usb/generic.txt
- snps,host-dma-disable: disable host DMA mode.
-- snps,gadget-dma-disable: disable gadget DMA mode.
-- snps,gadget-dma-desc-disable: disable gadget DMA descriptor mode.
- g-rx-fifo-size: size of rx fifo size in gadget mode.
- g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
- g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..2f18a7b 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1091,22 +1091,17 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
struct dwc2_hw_params *hw = &hsotg->hw_params;
struct dwc2_core_params *p = &hsotg->params;
bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
- bool disable;
/* Buffer DMA */
- disable = device_property_read_bool(hsotg->dev,
- "snps,gadget-dma-disable");
dwc2_set_param_bool(hsotg, &p->g_dma,
false, "gadget-dma",
- !disable, false,
+ true, false,
dma_capable);
/* DMA Descriptor */
- disable = device_property_read_bool(hsotg->dev,
- "snps,gadget-dma-desc-disable");
dwc2_set_param_bool(hsotg, &p->g_dma_desc, false,
"gadget-dma-desc",
- p->g_dma && !disable, false,
+ p->g_dma, false,
!!hw->dma_desc_enable);
}
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/3] usb: dwc2: params revert and rework
From: John Youn @ 2016-11-15 2:29 UTC (permalink / raw)
To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
Hi Felipe,
This reverts and fixes a few commits that are queued on your
testing/next, removing the previously added DT bindings, and the code
that reads them in.
The feedback was that IP validation is not reason enough to add these.
So we'll leave them out for now.
Regards,
John
John Youn (3):
Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
Revert "Documentation: devicetree: dwc2: Add host DMA binding"
usb: dwc2: Remove reading in of invalid property
Documentation/devicetree/bindings/usb/dwc2.txt | 3 ---
drivers/usb/dwc2/params.c | 16 +++-------------
2 files changed, 3 insertions(+), 16 deletions(-)
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Stephen Boyd @ 2016-11-15 2:13 UTC (permalink / raw)
To: Rob Herring
Cc: Viresh Kumar, Mark Brown, Rafael Wysocki, nm, Viresh Kumar,
linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, d-gerlach,
devicetree
In-Reply-To: <20161115015952.steomn4azdl5iv6z@rob-hp-laptop>
On 11/14, Rob Herring wrote:
> On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> > On 10-11-16, 14:51, Stephen Boyd wrote:
> > >
> > > No. The supply names (and also clock names/index) should be left
> > > up to the consumer of the OPP table. We don't want to encode any
> > > sort of details like this between the OPP table and the consumer
> > > of it in DT because then it seriously couples the OPP table to
> > > the consumer device. "The binding" in this case that needs to be
> > > updated is the consumer binding, to indicate that it correlated
> > > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > > voltages.
> >
> > Are you saying that we shall have a property like this then?
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index ee91cbdd95ee..733946df2fb8 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
> > compatible = "arm,cortex-a7";
> > ...
> >
> > - cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > + vcc0-supply = <&cpu_supply0>;
> > + vcc1-supply = <&cpu_supply1>;
> > + vcc2-supply = <&cpu_supply2>;
> > + opp-supply-names = "vcc0", "vcc1", "vcc2";
>
> Uh, no. You already have the names in the *-supply properties. Yes, they
> are a PIA to retrieve compared to a *-names property, but that is the
> nature of this style of binding.
>
I think the problem is that Viresh wants the binding to be "self
describing" so that the OPP can be used without a driver knowing
that a supply corresponds to a particular column in the voltage
table. I don't understand that though. Can't we set the supply
names from C code somewhere based on the consumer of the OPPs?
Similar to how we pick the different tables based on fuses?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Laurent Pinchart @ 2016-11-15 2:11 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Laurent Pinchart, dri-devel,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
linux-media@vger.kernel.org
In-Reply-To: <CAL_JsqL_41VGf8ZFEzQyqPU_EeK=Ms+jdt_N9cvRyLFkXF2MTg@mail.gmail.com>
Hi Rob,
On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>>> Multiple incompatible data link layers have been used over time to
> >>>>> transmit image data to LVDS panels. This binding supports display
> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>>> specifications.
> >>>>>
> >>>>> Signed-off-by: Laurent Pinchart
> >>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>> ---
> >>>>>
> >>>>> .../bindings/display/panel/panel-lvds.txt | 119 ++++++++++++
> >>>>> 1 file changed, 119 insertions(+)
> >>>>> create mode 100644
> >>>>> Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>>
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..250861f2673e
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> @@ -0,0 +1,119 @@
> >>>>> +Generic LVDS Panel
> >>>>> +==================
> >>>>> +
> >>>>> +LVDS is a physical layer specification defined in
> >>>>> ANSI/TIA/EIA-644-A. Multiple
> >>>>> +incompatible data link layers have been used over time to transmit
> >>>>> image data
> >>>>> +to LVDS panels. This bindings supports display panels compatible
> >>>>> with the
> >>>>> +following specifications.
> >>>>> +
> >>>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>>> February
> >>>>> +1999 (Version 1.0), Japan Electronic Industry Development
> >>>>> Association (JEIDA)
> >>>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>>> National
> >>>>> +Semiconductor
> >>>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>>> Video
> >>>>> +Electronics Standards Association (VESA)
> >>>>> +
> >>>>> +Device compatible with those specifications have been marketed under
> >>>>> the
> >>>>> +FPD-Link and FlatLink brands.
> >>>>> +
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: shall contain "panel-lvds"
> >>>>
> >>>> Maybe as a fallback, but on its own, no way.
> >>>
> >>> Which brings an interesting question: when designing generic DT
> >>> bindings, what's the rule regarding
> >
> > Looks like I forgot part of the question. I meant to ask what is the rule
> > regarding usage of more precise compatible strings ?
>
> When in doubt, always have one. If there's any chance at all that s/w
> will need to know or care, then we should have one.
>
> > For instance (but perhaps not the best example), the
> > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> > string, with no other bindings defining more precise compatible strings
> > for the exact rotary encoder model. When it comes to panels I believe it
> > makes sense to define model-specific compatible strings even if they're
> > unused by drivers. I'm however wondering what the rule is there, and
> > where those device-specific compatible strings should be defined. I'd
> > like to avoid using one file per panel model as done today for the
> > simple-panel bindings.
>
> There's a few exceptions like this where there is not any sort of
> model to base a compatible on. For example, a GPIO connected LED is
> truly generic. The only way to have a more specific compatible would
> be something with the board name in it.
>
> Your case here is in the middle. It seems like it's generic and
> passive, but perhaps power control is not. Rather than trying to
> decide, we can just cover our ass and put both a generic and specific
> compatible in.
That sounds good to me. I'll mention in the document that a more precise
compatible is required.
> >> Call it "simple" so I can easily NAK it. :)
> >>
> >> Define a generic structure, not a single binding trying to serve all.
> >>
> >>>>> +- width-mm: panel display width in millimeters
> >>>>> +- height-mm: panel display height in millimeters
> >>>>
> >>>> This is already documented for all panels IIRC.
> >>>
> >>> Note that this DT binding has nothing to do with the simple-panel
> >>> binding. It is instead similar to the panel-dpi and panel-dsi-cm
> >>> bindings (which currently don't but should specify the panel size in
> >>> DT). The LVDS panel driver will *not* include any panel-specific
> >>> information such as size or timings, these are specified in DT.
> >>
> >> The panel bindings aren't really different. The biggest difference was
> >> location in the tree, but we now generally allow panels to be either a
> >> child of the LCD controller or connected with OF graph. We probably
> >> need to work on restructuring the panel bindings a bit. We should have
> >> an inheritance with a base panel binding of things like size, label,
> >> graph, backlight, etc, then perhaps an interface specific bindings for
> >> LVDS, DSI, and parallel, then a panel specific binding. With this the
> >> panel specific binding is typically just a compatible string and which
> >> inherited properties apply to it.
> >
> > That sounds good to me, but we have multiple models for panel bindings.
> >
> > As you mentioned panels can be referenced through an LCD controller node
> > property containing a phandle to the panel node, or through OF graph.
> > That's a situation we have today, and we need to keep supporting both (at
> > least for existing panels, perhaps not for the new ones).
> >
> > Another difference is how to express panel data such as size and timings.
> > The simple-panel DT bindings don't contain such data and expects the
> > drivers to contain a table of panel data for all models supported, while
> > the DPI, DSI and now the proposed LVDS panel bindings contain properties
> > for panel data.
> >
> > How would you like to reconcile all that ?
>
> Thierry has outlined the position[1] that simple-panel follows many
> times and I generally agree. I could be convinced that perhaps panel
> timings could go into DT. However, you can't really describe
> *everything*, so we're not going to get away from panel specific
> compatible strings and panel specifics in the kernel.
I'm fine with that middle ground, and will include panel timings in DT but not
an information related to power sequencing or other panel-specific properties
(beside the standardized properties defined in this binding of course).
> >>>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>>> "jeida-24"
> >>>>> + or "vesa-24"
> >>>>
> >>>> Maybe this should be part of the compatible.
> >>>
> >>> I've thought about it, but given that some panels support selecting
> >>> between multiple modes (through a mode pin that is usually hardwired),
> >>> I believe a separate DT property makes sense.
> >>
> >> Okay.
> >>
> >>> Furthermore, LVDS data organization is controlled by the combination of
> >>> both data-mapping and data-mirror. It makes little sense from my point
> >>> of view to handle one as part of the compatible string and the other one
> >>> as a separate property.
> >>>
> >>>> > +Optional properties:
> >>>> > +- label: a symbolic name for the panel
> >>>>
> >>>> Could be for any panel or display connector.
> >>>
> >>> Yes, but I'm not sure to understand how that's relevant :-)
> >>
> >> Meaning it should be a common property.
> >
> > Sure. So you expect me to reorganize all the panels and connectors DT
> > bindings in order to get this one merged ? :-)
>
> No, because I don't think label is widely defined. Just put it in a
> common place and reference it. Any other panels can be fixed later.
> Really, the "simple panel" binding should probably morph into the
> common binding.
The "label" property is actually defined in the "Devicetree Specification,
Release 0.1" as recently published on devicetree.org. Where would you like me
to define it in the bindings ?
> >>>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>>> analog supply
> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>>> digital supply
> >>>>
> >>>> Which one has to be powered on first, what voltage, and with what time
> >>>> in between? This is why "generic" or "simple" bindings don't work.
> >>>
> >>> The above-mentioned specifications also define connectors, pinouts and
> >>> power supplies, but many LVDS panels compatible with the LVDS physical
> >>> and data layers use a different connector with small differences in
> >>> power supplies.
> >>>
> >>> I believe the voltage is irrelevant here, it doesn't need to be
> >>> controlled by the operating system. Power supplies order and timing is
> >>> relevant, I'll investigate the level of differences between panels. I'm
> >>> also fine with dropping those properties for now.
> >>
> >> Whether you have control of the supplies is dependent on the board.
> >> Dropping them is just puts us in the simple binding trap. The simple
> >> bindings start out that way and then people keep adding to them.
> >
> > Damn, you can't be fooled easily ;-)
>
> I guess you can count all the simple bindings to see how many times I
> can be fooled. :)
>
> > On a more serious note, I'd like to design the bindings in a way that
> > wouldn't require adding device-specific code in the driver for each panel
> > model, given that in most cases power supply handling will be generic.
> > What's your opinion about a generic power supply model that would be used
> > in the default case, with the option to override it with device-specific
> > code when needed ?
>
> I don't agree. Read Thierry's post on the subject[1].
I'm not sure you understood me correctly (and writing a clarification at
4:00am might not help :-)). My point here is that a fair number of panels
don't care about power sequencing and have no control GPIOs (that's certainly
the case of the two Mitsubishi panels I use here that have a single power
supply - good luck trying to convince me that this needs to be sequenced :-) -
and no enable or reset control pin). This kind of panel should obviously be
modelled in DT with both a specific and a generic ("panel-lvds") compatible
string, to ensure that we'll have enough information available on the
operating system side to control the panel properly. The power supply(ies)
should be documented in relation to the specific compatible string and not
mentioned by the generic bindings (whether that should go in one or multiple
text files is bikeshedding).
This being said, on the driver side, I don't see a reason to list the specific
compatible strings explicitly for those panels when a generic implementation
matching the generic compatible string can handle them fine. This has no
impact on the bindings and is an Linux implementation decision.
> [1]
> http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Rob Herring @ 2016-11-15 1:59 UTC (permalink / raw)
To: Viresh Kumar
Cc: Stephen Boyd, Mark Brown, Rafael Wysocki, nm-l0cyMroinI0,
Viresh Kumar, linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
d-gerlach-l0cyMroinI0, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161111031120.GE11670@vireshk-i7>
On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> On 10-11-16, 14:51, Stephen Boyd wrote:
> > On 11/10, Viresh Kumar wrote:
> > > On 10-11-16, 16:36, Mark Brown wrote:
> > > > On Thu, Nov 10, 2016 at 09:34:40AM +0530, Viresh Kumar wrote:
> > > > > On 09-11-16, 14:58, Mark Brown wrote:
> > > > > > On Wed, Oct 26, 2016 at 12:02:56PM +0530, Viresh Kumar wrote:
> > > >
> > > > > > > + Entries for multiple regulators shall be provided in the same field separated
> > > > > > > + by angular brackets <>. The OPP binding doesn't provide any provisions to
> > > > > > > + relate the values to their power supplies or the order in which the supplies
> > > > > > > + need to be configured.
> > > >
> > > > > > I don't understand how this works. If we have an unordered list of
> > > > > > values to set for regulators how will we make sense of them?
> > > >
> > > > > The platform driver is responsible to identify the order and pass it on to the
> > > > > OPP core. And the platform driver needs to have that hard coded.
> > > >
> > > > That *really* should be in the binding.
> > >
> > > Okay, how do you suggest doing that? Will a property like supply-names
> > > in the OPP table be fine? Like this:
> > >
> > > @@ -369,13 +378,16 @@ Example 4: Handling multiple regulators
> > > compatible = "arm,cortex-a7";
> > > ...
> > >
> > > - cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > > + vcc0-supply = <&cpu_supply0>;
> > > + vcc1-supply = <&cpu_supply1>;
> > > + vcc2-supply = <&cpu_supply2>;
> > > operating-points-v2 = <&cpu0_opp_table>;
> > > };
> > > };
> > >
> > > cpu0_opp_table: opp_table0 {
> > > compatible = "operating-points-v2";
> > > + supply-names = "vcc0", "vcc1", "vcc2";
> > > opp-shared;
> > >
> >
> > No. The supply names (and also clock names/index) should be left
> > up to the consumer of the OPP table. We don't want to encode any
> > sort of details like this between the OPP table and the consumer
> > of it in DT because then it seriously couples the OPP table to
> > the consumer device. "The binding" in this case that needs to be
> > updated is the consumer binding, to indicate that it correlated
> > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > voltages.
>
> Are you saying that we shall have a property like this then?
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index ee91cbdd95ee..733946df2fb8 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
> compatible = "arm,cortex-a7";
> ...
>
> - cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> + vcc0-supply = <&cpu_supply0>;
> + vcc1-supply = <&cpu_supply1>;
> + vcc2-supply = <&cpu_supply2>;
> + opp-supply-names = "vcc0", "vcc1", "vcc2";
Uh, no. You already have the names in the *-supply properties. Yes, they
are a PIA to retrieve compared to a *-names property, but that is the
nature of this style of binding.
Rob
--
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: hisilicon: add CRG driver for Hi3516CV300 SoC
From: Jiancheng Xue @ 2016-11-15 1:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: mturquette-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
bin.chen-QSEj5FYQhm4dnm+yROfE0A, elder-QSEj5FYQhm4dnm+yROfE0A,
hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q,
yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q, wenpan-C8/M+/jPZTeaMJb+Lgu22Q,
howell.yang-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161114200359.GM5177-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 2016/11/15 4:03, Stephen Boyd wrote:
> On 11/14, Jiancheng Xue wrote:
>> Hi Stephen,
>>
>> On 2016/11/12 8:04, Stephen Boyd wrote:
>>> On 10/29, Jiancheng Xue wrote:
>>>
>>> Should be a From: Pan Wen here?
>>>
>>>> Add CRG driver for Hi3516CV300 SoC. CRG(Clock and Reset
>>>> Generator) module generates clock and reset signals used
>>>> by other module blocks on SoC.
>>>>
>>>> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>>
>>> And you should have signed it off? Care to resend or state that
>>> this is incorrectly attributed to you instead of Pan Wen?
>>>
>>
>> Pan Wen is the main author of this patch. I just made some small modification
>> with agreement from him. Do I need to resend this patch if it's better to add my
>> signed-off?
>>
>
> Sure I'll apply the resent one. But you should also make sure to
> retain Pan Wen's authorship. I'll go do that when applying it.
>
OK. I have resent this patch. Please check it. Thank you, Stephen.
Regards,
Jiancheng
--
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 2/2] of: changesets: Introduce changeset helper methods
From: Frank Rowand @ 2016-11-15 1:56 UTC (permalink / raw)
To: Rob Herring
Cc: Hans de Goede, Pantelis Antoniou,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+zWiXOtb4hWrpB87z8T4WLfCbLeGNgST4tmAz61dgFHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/14/16 14:16, Rob Herring wrote:
> On Mon, Nov 14, 2016 at 12:44 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 11/14/16 03:04, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 14-11-16 08:34, Frank Rowand wrote:
>>>> Hi Hans, Pantelis,
>>>>
>>>> On 11/12/16 18:15, Frank Rowand wrote:
>>>>> On 11/04/16 07:42, Hans de Goede wrote:
>>>>>> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>>
>>>>>> Changesets are very powerful, but the lack of a helper API
>>>>>> makes using them cumbersome. Introduce a simple copy based
>>>>>> API that makes things considerably easier.
>>>>>>
>>>>>> To wit, adding a property using the raw API.
>>>>>>
>>>>>> struct property *prop;
>>>>>> prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
>>>>>> prop->name = kstrdup("compatible");
>>>>>> prop->value = kstrdup("foo,bar");
>>>>>> prop->length = strlen(prop->value) + 1;
>>>>>> of_changeset_add_property(ocs, np, prop);
>>>>>>
>>>>>> while using the helper API
>>>>>>
>>>>>> of_changeset_add_property_string(ocs, np, "compatible",
>>>>>> "foo,bar");
>>>>>>
>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>> Changes in v2 (hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
>>>>>> -Address review comments from:
>>>>>> https://www.spinics.net/lists/kernel/msg2252845.html
>>>>>
>>>>> That points to the May 9 version 1 patches from Pantelis (as expected),
>>>>> but containing 4, not 2, patches. Patch 1/4 was applied. Patch 4/4
>>>>> seems to have disappeared?
>>>>>
>>>>> Pantelis then sent a version 2 set of the patches on May 16.
>>>>>
>>>>> Your version is a modification of the May 9 patches (as would be expected
>>>>> of a version 2). It is confusing to have two different version 2 patch
>>>>> sets. I don't have any brilliant ideas on how this patch set could have
>>>>> been named differently to avoid that confusion.
>>>>>
>>>>> The point of this little side-track is simply to note the existence of two
>>>>> different version 2 series so I won't be confused when I revisit this
>>>>> thread in the future.
>>>>>
>>>>>> -Simplify (and fix) __of_changeset_add_update_property_copy OOM handling
>>>>>> -Remove (by manual inlining) these 2 static helpers:
>>>>>> __of_changeset_add_update_property_u32
>>>>>> __of_changeset_add_update_property_bool
>>>>>> -Remove the following exported helper method:
>>>>>> of_changeset_node_move_to
>>>>>
>>>>> Not all comments were addressed.
>>>>>
>>>>> There are some other changes made that are not noted in the changelog.
>>>>>
>>>>> I am still reading through the patches. I will reply again either with
>>>>> a reviewed-by or specific comments when I finish.
>>>>
>>>> Replying here for the entire patchset (there was no patch 0 to reply to).
>>>>
>>>> After reading through the patches, my reply is meta instead of specific
>>>> comments about the code.
>>>>
>>>> There are very few users of change sets in tree. I do not see the need to
>>>> add these helpers until such users are likely to appear.
>>>>
>>>> I would expect change sets to be _mostly_ used internally by the device tree
>>>> overlay framework, not directly by drivers. If change sets are an attractive
>>>> technology for drivers, I want to approach that usage very carefully to avoid
>>>> inappropriate use, which could be very difficult to reign in after the fact.
>>>>
>>>> Even if helpers should be added, this seems to be an overly complex approach.
>>>> If the need for these helpers becomes apparent I can provide review comments
>>>> with the specifics about how it appears to be overly complex.
>>>>
>>>> Can you please provide some more insights into the needs driving the desire
>>>> to have change set helpers and the expected use cases of them? Please put
>>>> your architect's hat on when replying to this question.
>>>
>>> My use case for this is discussed in this thread:
>>> https://www.spinics.net/lists/arm-kernel/msg536111.html
>>>
>>> With the dt-bindings for the hardware-manager I want to add here:
>>> https://www.spinics.net/lists/arm-kernel/msg536109.html
>>>
>>> Note that there is a lot of discussion in this thread whether or
>>> not this belongs in the kernel. I strongly believe though that
>>> some functionality like this will be needed in the kernel for
>>> ARM+dt devices going forward, just like there is plenty of x86
>>> code which adjusts itself to specific hardware, because whether
>>> we like it or not hardware (revisions) will always have quirks.
>>
>> Thanks! That context should have been provided with the patches.
>>
>> The use case discussion is important and I am paying a lot of
>> attention to that discussion and many other discussions about
>> dynamic device trees. I don't think it makes sense to apply the
>> change set helper patches yet, given the unsettled state of the
>> various dynamic device tree discussions.
>
> These helpers are useful and easier to use than the existing API
> independent of any issues to sort out with how we use overlays. So I
> plan to take them whether there's a user right away or not.
>
> Rob
OK, expect a more detailed review from me this week.
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] dt/binding: ti-tsc-adc: deprecate ti,charge-delay and add binding doc for ti,charge-delay-ns
From: Rob Herring @ 2016-11-15 1:54 UTC (permalink / raw)
To: Mugunthan V N
Cc: linux-input, Dmitry Torokhov, Jonathan Cameron, Mark Rutland,
Lee Jones, Sekhar Nori, Vignesh R, devicetree, linux-omap,
linux-kernel
In-Reply-To: <20161110163515.27598-2-mugunthanvnm@ti.com>
On Thu, Nov 10, 2016 at 10:05:13PM +0530, Mugunthan V N wrote:
> ti,charge-delay represents the duration that ADC should wait
> before sampling the ADC line to detect the touch location and pen
> up/downs. Currently the ADC clock is set at 3MHz. The device-tree
> entry for ti,charge-delay is based on assumption of ADC clock at
> 3MHz, but it can be operated up to 24MHz clock. Representing the
> charge delay of touchscreen in terms of ADC clocks is incorrect.
> So change this representation to ti,charge-delay-ns, which driver
> can convert it to number clock cycles based on ref clock
> frequency.
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> .../bindings/input/touchscreen/ti-tsc-adc.txt | 32 ++++++++++++++--------
> 1 file changed, 21 insertions(+), 11 deletions(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/2] devicetree: Add vendor prefix for CZ.NIC
From: Rob Herring @ 2016-11-15 1:49 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
Martin Strbačka, Tomas Hlavacek, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161110135721.13098-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
On Thu, Nov 10, 2016 at 02:57:20PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
> ---
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@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
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