* [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Sergey Suloev @ 2018-05-18 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4909574.Q3IFWM0xt6@jernej-laptop>
Hi, guys,
On 05/18/2018 05:46 PM, Jernej ?krabec wrote:
> Hi,
>
> Dne petek, 18. maj 2018 ob 12:01:16 CEST je Maxime Ripard napisal(a):
>> On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:
>>> From: Jernej Skrabec <jernej.skrabec@siol.net>
>>>
>>> Some SoCs with DW HDMI have multiple possible clock parents, like A64
>>> and R40.
>>>
>>> Expand HDMI PHY clock driver to support second clock parent.
>>>
>>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>> Changes for v2:
>>> - new patch
>>>
>>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 9 ++-
>>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 33 ++++++++---
>>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89
>>> ++++++++++++++++++++++-------- 3 files changed, 96 insertions(+), 35
>>> deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..303189d6602c
>>> 100644
>>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>> @@ -98,7 +98,8 @@
>>>
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29)
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28)
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)
>>>
>>> -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26)
>>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
>>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26
>>>
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22)
>>> #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20)
>>>
>>> @@ -146,7 +147,7 @@
>>>
>>> struct sun8i_hdmi_phy;
>>>
>>> struct sun8i_hdmi_phy_variant {
>>>
>>> - bool has_phy_clk;
>>> + int phy_clk_num;
>>>
>>> void (*phy_init)(struct sun8i_hdmi_phy *phy);
>>> void (*phy_disable)(struct dw_hdmi *hdmi,
>>>
>>> struct sun8i_hdmi_phy *phy);
>>>
>>> @@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {
>>>
>>> struct clk *clk_mod;
>>> struct clk *clk_phy;
>>> struct clk *clk_pll0;
>>>
>>> + struct clk *clk_pll1;
>>>
>>> unsigned int rcal;
>>> struct regmap *regs;
>>> struct reset_control *rst_phy;
>>>
>>> @@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
>>> *hdmi);
>>>
>>> void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
>>> const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
>>>
>>> -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev);
>>> +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev,
>>> + int clk_num);
>>>
>>> #endif /* _SUN8I_DW_HDMI_H_ */
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 5a52fc489a9d..0eadf087fc46
>>> 100644
>>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>> @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
>>> *hdmi,>
>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
>>>
>>> SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
>>>
>>> - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
>>> + /*
>>> + * NOTE: We have to be careful not to overwrite PHY parent
>>> + * clock selection bit and clock divider.
>>> + */
>>> + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
>>> + (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
>>> + pll_cfg1_init);
>>>
>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
>>>
>>> (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
>>> pll_cfg2_init);
>>>
>>> @@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi *hdmi,
>>> void *data,>
>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
>>>
>>> SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
>>>
>>> - if (phy->variant->has_phy_clk)
>>> + if (phy->variant->phy_clk_num)
>>>
>>> clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);
>>>
>>> return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);
>>>
>>> @@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant
>>> sun8i_a83t_hdmi_phy = {>
>>> };
>>>
>>> static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
>>>
>>> - .has_phy_clk = true,
>>> + .phy_clk_num = 1,
>>>
>>> .phy_init = &sun8i_hdmi_phy_init_h3,
>>> .phy_disable = &sun8i_hdmi_phy_disable_h3,
>>> .phy_config = &sun8i_hdmi_phy_config_h3,
>>>
>>> @@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
>>> struct device_node *node)>
>>> goto err_put_clk_bus;
>>>
>>> }
>>>
>>> - if (phy->variant->has_phy_clk) {
>>> + if (phy->variant->phy_clk_num) {
>>>
>>> phy->clk_pll0 = of_clk_get_by_name(node, "pll-0");
>>> if (IS_ERR(phy->clk_pll0)) {
>>>
>>> dev_err(dev, "Could not get pll-0 clock\n");
>>>
>>> @@ -472,7 +478,16 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
>>> struct device_node *node)>
>>> goto err_put_clk_mod;
>>>
>>> }
>>>
>>> - ret = sun8i_phy_clk_create(phy, dev);
>>> + if (phy->variant->phy_clk_num) {
>>> + phy->clk_pll1 = of_clk_get_by_name(node, "pll-1");
>>> + if (IS_ERR(phy->clk_pll1)) {
>>> + dev_err(dev, "Could not get pll-1 clock\n");
>>> + ret = PTR_ERR(phy->clk_pll1);
>>> + goto err_put_clk_mod;
>>> + }
>>> + }
>>> +
>> You have a bug here. If phy_clk_num == 1, you'll still try to lookup
>> pll-1.
> This is actually WIP patch taken from my github. This issue was fixed already
> locally on disk. I thought Jagan will not use it until SRAM C patches land.
>
>> And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
>> lookup pll-2 either.
> It is highly unlikely this will be higher than 2, at least for this HDMI PHY,
> since it has only 1 bit reserved for parent selection. But since I have to fix
> it, I'll add ">= 2"
>
>>> + ret = sun8i_phy_clk_create(phy, dev, phy->variant->phy_clk_num);
>>>
>>> if (ret) {
>>>
>>> dev_err(dev, "Couldn't create the PHY clock\n");
>>> goto err_put_clk_pll0;
>>>
>>> @@ -515,8 +530,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
>>> struct device_node *node)>
>>> err_put_rst_phy:
>>> reset_control_put(phy->rst_phy);
>>>
>>> err_put_clk_pll0:
>>> - if (phy->variant->has_phy_clk)
>>> - clk_put(phy->clk_pll0);
>>> + clk_put(phy->clk_pll0);
>>> + clk_put(phy->clk_pll1);
>>>
>>> err_put_clk_mod:
>>> clk_put(phy->clk_mod);
>>>
>>> err_put_clk_bus:
>>> @@ -536,8 +551,8 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi)
>>>
>>> reset_control_put(phy->rst_phy);
>>>
>>> - if (phy->variant->has_phy_clk)
>>> - clk_put(phy->clk_pll0);
>>> + clk_put(phy->clk_pll0);
>>> + clk_put(phy->clk_pll1);
>>>
>>> clk_put(phy->clk_mod);
>>> clk_put(phy->clk_bus);
>>>
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c index
>>> faea449812f8..85b12fc96dbc 100644
>>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>> @@ -22,29 +22,36 @@ static int sun8i_phy_clk_determine_rate(struct clk_hw
>>> *hw,>
>>> {
>>>
>>> unsigned long rate = req->rate;
>>> unsigned long best_rate = 0;
>>>
>>> - struct clk_hw *parent;
>>> + struct clk_hw *best_parent = NULL;
>>> + struct clk_hw *parent = NULL;
>>>
>>> int best_div = 1;
>>>
>>> - int i;
>>> + int i, p;
>>>
>>> - parent = clk_hw_get_parent(hw);
>>> -
>>> - for (i = 1; i <= 16; i++) {
>>> - unsigned long ideal = rate * i;
>>> - unsigned long rounded;
>>> -
>>> - rounded = clk_hw_round_rate(parent, ideal);
>>> -
>>> - if (rounded == ideal) {
>>> - best_rate = rounded;
>>> - best_div = i;
>>> - break;
>>> - }
>>> + for (p = 0; p < clk_hw_get_num_parents(hw); p++) {
>>> + parent = clk_hw_get_parent_by_index(hw, p);
>>> + if (!parent)
>>> + continue;
>>>
>>> - if (!best_rate ||
>>> - abs(rate - rounded / i) <
>>> - abs(rate - best_rate / best_div)) {
>>> - best_rate = rounded;
>>> - best_div = i;
>>> + for (i = 1; i <= 16; i++) {
>>> + unsigned long ideal = rate * i;
>>> + unsigned long rounded;
>>> +
>>> + rounded = clk_hw_round_rate(parent, ideal);
>>> +
>>> + if (rounded == ideal) {
>>> + best_rate = rounded;
>>> + best_div = i;
>>> + best_parent = parent;
>>> + break;
>>> + }
>>> +
>>> + if (!best_rate ||
>>> + abs(rate - rounded / i) <
>>> + abs(rate - best_rate / best_div)) {
>>> + best_rate = rounded;
>>> + best_div = i;
>>> + best_parent = parent;
>>> + }
>>>
>>> }
>>>
>>> }
>>>
>>> @@ -95,22 +102,58 @@ static int sun8i_phy_clk_set_rate(struct clk_hw *hw,
>>> unsigned long rate,>
>>> return 0;
>>>
>>> }
>>>
>>> +static u8 sun8i_phy_clk_get_parent(struct clk_hw *hw)
>>> +{
>>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
>>> + u32 reg;
>>> +
>>> + regmap_read(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, ®);
>>> + reg = (reg & SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK) >>
>>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT;
>>> +
>>> + return reg;
>>> +}
>>> +
>>> +static int sun8i_phy_clk_set_parent(struct clk_hw *hw, u8 index)
>>> +{
>>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
>>> +
>>> + if (index > 1)
>>> + return -EINVAL;
>>> +
>>> + regmap_update_bits(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
>>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
>>> + index << SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT);
>>> +
>>> + return 0;
>>> +}
>>> +
>> The DT bindings changes and the clk changes should be part of separate
>> patches.
> By DT bindings changes you mean code which reads DT and not DT documentation,
> right?
>
> Ok, I'll split it.
>
> BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
> there is nothing to hold it back, unlike for this.
>
> Best regards,
> Jernej
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
you have been talking about SRAM patches, required for A64 DE2, for
about a half a year.
May I ask you to explain in a couple of words why they are so important ?
I am really curious because I have DE2 already working on my A64 without
those magic patches..
Thanks,
Sergey
^ permalink raw reply
* [GIT PULL] ARM: mediatek: soc driver updates for v4.18
From: Matthias Brugger @ 2018-05-18 15:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180517044600.GC20254@sirena.org.uk>
On 17/05/18 06:46, Mark Brown wrote:
> On Mon, May 14, 2018 at 01:32:49PM +0200, Matthias Brugger wrote:
>
>> ----------------------------------------------------------------
>> Matthias Brugger (1):
>> Merge commit 'f15cd6d99198e9c15229aefec639a34a6e8174c6' into
>> v.4.17-next/soc-test
>
> There is a signed tag for this - please don't ever pull anything from me
> without a signed tag, while I try to avoid rewriting history it does
> sometimes happen but if I've made a signed tag part of that is me saying
> I know other people might've merged the commit.
>
You are right I got confused about this.
Anrd, Olof do you want me to resubmit?
Regards,
Matthias
^ permalink raw reply
* [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input
From: Laurent Pinchart @ 2018-05-18 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526654878-11143-3-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thank you for the patch.
On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> Describe CVBS video input through analog video decoder ADV7180
> connected to video input interface VIN4.
>
> The video input signal path is shared with HDMI video input, and
> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> by the default switches configuration.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
>
> ---
> v2 -> v3:
> - Add comment to describe the shared input video path
> - Add my SoB and Niklas' R-b tags
> ---
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 +++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..95745fc
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -142,6 +142,11 @@
> groups = "usb0";
> function = "usb0";
> };
> +
> + vin4_pins_cvbs: vin4 {
> + groups = "vin4_data8", "vin4_sync", "vin4_clk";
> + function = "vin4";
> + };
> };
>
> &i2c0 {
> @@ -154,6 +159,23 @@
> reg = <0x50>;
> pagesize = <8>;
> };
> +
> + analog-video at 20 {
> + compatible = "adi,adv7180";
> + reg = <0x20>;
> +
> + port {
The adv7180 DT bindings document the output port as 3 or 6 (respectively for
the CP and ST versions of the chip). You should thus number the port. Apart
from that the patch looks good.
> + /*
> + * The VIN4 video input path is shared between
> + * CVBS and HDMI inputs through SW[49-54] switches.
> + *
> + * CVBS is the default selection, link it to VIN4 here.
> + */
> + adv7180_out: endpoint {
> + remote-endpoint = <&vin4_in>;
> + };
> + };
> + };
> };
>
> &i2c1 {
> @@ -246,3 +268,23 @@
> timeout-sec = <60>;
> status = "okay";
> };
> +
> +&vin4 {
> + pinctrl-0 = <&vin4_pins_cvbs>;
> + pinctrl-names = "default";
> +
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> +
> + vin4_in: endpoint {
> + remote-endpoint = <&adv7180_out>;
> + };
> + };
> + };
> +};
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v3 3/3] arm64: dts: renesas: draak: Describe HDMI input
From: Laurent Pinchart @ 2018-05-18 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526654878-11143-4-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thank you for the patch.
On Friday, 18 May 2018 17:47:58 EEST Jacopo Mondi wrote:
> Describe HDMI input connector and ADV7612 HDMI decoder installed on
> R-Car Gen3 Draak board.
>
> The video signal routing to the HDMI decoder to the video input interface
> VIN4 is multiplexed with CVBS input path, and enabled/disabled through
> on-board switches SW-49, SW-50, SW-51 and SW-52.
>
> As the default board switches configuration connects CVBS input to VIN4,
> leave the HDMI decoder unconnected in DTS.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v2 -> v3:
> - Add comment on HDMI output port about the shared CVBS/HDMI video path
> - Add Niklas' R-b tag
> ---
> arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 48 +++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 95745fc..1e475a4
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,17 @@
> };
> };
>
> + hdmi-in {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con_in: endpoint {
> + remote-endpoint = <&adv7612_in>;
> + };
> + };
> + };
> +
> memory at 48000000 {
> device_type = "memory";
> /* first 128MB is reserved for secure area. */
> @@ -176,6 +187,43 @@
> };
> };
> };
> +
> + hdmi-decoder at 4c {
> + compatible = "adi,adv7612";
> + reg = <0x4c>;
> + default-input = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port at 0 {
> + reg = <0>;
> +
> + adv7612_in: endpoint {
> + remote-endpoint = <&hdmi_con_in>;
> + };
> + };
> +
> + port at 2 {
> + reg = <2>;
> +
> + /*
> + * The VIN4 video input path is shared between
> + * CVBS and HDMI inputs through SW[49-54]
> + * switches.
> + *
> + * CVBS is the default selection, leave HDMI
> + * not connected here.
> + */
> + adv7612_out: endpoint {
> + pclk-sample = <0>;
> + hsync-active = <0>;
> + vsync-active = <0>;
> + };
> + };
> + };
> + };
> };
>
> &i2c1 {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [GIT PULL] ARM: mediatek: soc driver updates for v4.18
From: Mark Brown @ 2018-05-18 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3e416f8f-b038-9bfe-7038-98aec5224620@gmail.com>
On Fri, May 18, 2018 at 05:11:32PM +0200, Matthias Brugger wrote:
> On 17/05/18 06:46, Mark Brown wrote:
> > On Mon, May 14, 2018 at 01:32:49PM +0200, Matthias Brugger wrote:
> > > ----------------------------------------------------------------
> > > Matthias Brugger (1):
> > > Merge commit 'f15cd6d99198e9c15229aefec639a34a6e8174c6' into
> > > v.4.17-next/soc-test
> > There is a signed tag for this - please don't ever pull anything from me
> > without a signed tag, while I try to avoid rewriting history it does
> > sometimes happen but if I've made a signed tag part of that is me saying
> > I know other people might've merged the commit.
> You are right I got confused about this.
> Anrd, Olof do you want me to resubmit?
I'm OK with leaving things as is - that was more a request for the
future than a request to rewrite history (if it's already been merged).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180518/264bd0f3/attachment.sig>
^ permalink raw reply
* [PATCH 1/5] dt-bindings: pinctrl: document the STMFX pinctrl bindings
From: Amelie DELAUNAY @ 2018-05-18 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518135237.GQ5130@dell>
On 05/18/2018 03:52 PM, Lee Jones wrote:
> On Fri, 18 May 2018, Amelie DELAUNAY wrote:
>
>> On 05/17/2018 08:36 AM, Lee Jones wrote:
>>> On Wed, 16 May 2018, Amelie DELAUNAY wrote:
>>>
>>>>
>>>>
>>>> On 05/16/2018 04:20 PM, Linus Walleij wrote:
>>>>> On Wed, May 9, 2018 at 9:56 AM, Amelie DELAUNAY <amelie.delaunay@st.com> wrote:
>>>>>
>>>>>> Indeed, stmfx has other functions than GPIO. But, after comments done
>>>>>> here: [1] and there: [2], it has been decided to move MFD parent/GPIO
>>>>>> child drivers into a single PINCTRL/GPIO driver because of the following
>>>>>> reasons:
>>>>>> - Other stmfx functions (IDD measurement and TouchScreen controller) are
>>>>>> not used on any of the boards using an stmfx and supported by Linux, so
>>>>>> no way to test these functions, and no need to maintain them while they
>>>>>> are not being used.
>>>>>> - But, in the case a new board will use more than GPIO function on
>>>>>> stmfx, the actual implementation allow to easily extract common init
>>>>>> part of stmfx and put it in an MFD driver.
>>>>>>
>>>>>> So I could remove gpio sub-node and put its contents in stmfx node and
>>>>>> keep single PINCTRL/GPIO driver for the time being.
>>>>>> Please advise,
>>>>>
>>>>> I would normally advice to use the right modeling from the start, create
>>>>> the MFD driver and spawn the devices from there. It is confusing
>>>>> if the layout of the driver(s) doesn't really match the layout of the
>>>>> hardware.
>>>>>
>>>>> I understand that it is a pain to write new MFD drivers to get your
>>>>> things going and it would be "nice to get this working really quick
>>>>> now" but in my experience it is better to do it right from the start.
>>>>>
>>>>
>>>> Hi Linus,
>>>>
>>>> Thanks for your advice. I understand the point.
>>>> So, the right modeling would be to:
>>>> - create an MFD driver with the common init part of stmfx
>>>> - remove all common init part of stmfx-pinctrl driver and keep only all
>>>> gpio/pinctrl functions.
>>>>
>>>> I will not develop the other stmfx functions (IDD measurement driver and
>>>> TouchScreen controller driver) because, as explained ealier, they are
>>>> not used on any of the boards using an stmfx and supported by Linux, so
>>>> no way to test these functions, and no need to maintain them while they
>>>> are not being used.
>>>>
>>>> Lee, are you OK with that ?
>>>
>>> I missed a lot of this conversation I think, but from what I've read,
>>> it sounds fine.
>>>
>>
>> I summarize the situation:
>> - I still don't have an official datasheet for STMFX device which could
>> justify the use of an MFD driver;
>> - the MFD driver will contain the STMFX chip initialization stuff such
>> as regmap initialization (regmap structure will be shared with the
>> child), chip initialization, global interrupt management;
>> - there will be only one child (GPIO/PINCTRL node) for the time being.
>>
>> So, is "MFD driver + GPIO/PINCTRL driver" the right modeling, and does
>> it still sound fine after this summary ? :)
>
> It is starting to sound like there will only ever be one child device,
> which starts to cross the line into "this is not an MFD" (M = Multi)
> territory.
>
... for the time being. So, Linus, Lee, is it possible to find common
ground ?
^ permalink raw reply
* [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Jernej Škrabec @ 2018-05-18 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <824c6989-7930-86dc-1195-494580f6cb38@orpaltech.com>
Hi,
Dne petek, 18. maj 2018 ob 17:09:40 CEST je Sergey Suloev napisal(a):
> Hi, guys,
>
> On 05/18/2018 05:46 PM, Jernej ?krabec wrote:
> > Hi,
> >
> > Dne petek, 18. maj 2018 ob 12:01:16 CEST je Maxime Ripard napisal(a):
> >> On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:
> >>> From: Jernej Skrabec <jernej.skrabec@siol.net>
> >>>
> >>> Some SoCs with DW HDMI have multiple possible clock parents, like A64
> >>> and R40.
> >>>
> >>> Expand HDMI PHY clock driver to support second clock parent.
> >>>
> >>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>> ---
> >>> Changes for v2:
> >>> - new patch
> >>>
> >>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 9 ++-
> >>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 33 ++++++++---
> >>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89
> >>> ++++++++++++++++++++++-------- 3 files changed, 96 insertions(+), 35
> >>> deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> >>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..303189d6602c
> >>> 100644
> >>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> >>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> >>> @@ -98,7 +98,8 @@
> >>>
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29)
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28)
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)
> >>>
> >>> -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26)
> >>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
> >>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26
> >>>
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22)
> >>> #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20)
> >>>
> >>> @@ -146,7 +147,7 @@
> >>>
> >>> struct sun8i_hdmi_phy;
> >>>
> >>> struct sun8i_hdmi_phy_variant {
> >>>
> >>> - bool has_phy_clk;
> >>> + int phy_clk_num;
> >>>
> >>> void (*phy_init)(struct sun8i_hdmi_phy *phy);
> >>> void (*phy_disable)(struct dw_hdmi *hdmi,
> >>>
> >>> struct sun8i_hdmi_phy *phy);
> >>>
> >>> @@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {
> >>>
> >>> struct clk *clk_mod;
> >>> struct clk *clk_phy;
> >>> struct clk *clk_pll0;
> >>>
> >>> + struct clk *clk_pll1;
> >>>
> >>> unsigned int rcal;
> >>> struct regmap *regs;
> >>> struct reset_control *rst_phy;
> >>>
> >>> @@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
> >>> *hdmi);
> >>>
> >>> void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
> >>> const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
> >>>
> >>> -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
> >>> *dev);
> >>> +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
> >>> *dev,
> >>> + int clk_num);
> >>>
> >>> #endif /* _SUN8I_DW_HDMI_H_ */
> >>>
> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> >>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index
> >>> 5a52fc489a9d..0eadf087fc46
> >>> 100644
> >>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> >>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> >>> @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
> >>> *hdmi,>
> >>>
> >>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
> >>>
> >>> SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
> >>>
> >>> - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
> >>> + /*
> >>> + * NOTE: We have to be careful not to overwrite PHY parent
> >>> + * clock selection bit and clock divider.
> >>> + */
> >>> + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> >>> + (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> >>> + pll_cfg1_init);
> >>>
> >>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
> >>>
> >>> (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
> >>> pll_cfg2_init);
> >>>
> >>> @@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi
> >>> *hdmi,
> >>> void *data,>
> >>>
> >>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
> >>>
> >>> SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
> >>>
> >>> - if (phy->variant->has_phy_clk)
> >>> + if (phy->variant->phy_clk_num)
> >>>
> >>> clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);
> >>>
> >>> return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);
> >>>
> >>> @@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant
> >>> sun8i_a83t_hdmi_phy = {>
> >>>
> >>> };
> >>>
> >>> static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
> >>>
> >>> - .has_phy_clk = true,
> >>> + .phy_clk_num = 1,
> >>>
> >>> .phy_init = &sun8i_hdmi_phy_init_h3,
> >>> .phy_disable = &sun8i_hdmi_phy_disable_h3,
> >>> .phy_config = &sun8i_hdmi_phy_config_h3,
> >>>
> >>> @@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
> >>> struct device_node *node)>
> >>>
> >>> goto err_put_clk_bus;
> >>>
> >>> }
> >>>
> >>> - if (phy->variant->has_phy_clk) {
> >>> + if (phy->variant->phy_clk_num) {
> >>>
> >>> phy->clk_pll0 = of_clk_get_by_name(node, "pll-0");
> >>> if (IS_ERR(phy->clk_pll0)) {
> >>>
> >>> dev_err(dev, "Could not get pll-0 clock\n");
> >>>
> >>> @@ -472,7 +478,16 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi
> >>> *hdmi,
> >>> struct device_node *node)>
> >>>
> >>> goto err_put_clk_mod;
> >>>
> >>> }
> >>>
> >>> - ret = sun8i_phy_clk_create(phy, dev);
> >>> + if (phy->variant->phy_clk_num) {
> >>> + phy->clk_pll1 = of_clk_get_by_name(node, "pll-1");
> >>> + if (IS_ERR(phy->clk_pll1)) {
> >>> + dev_err(dev, "Could not get pll-1 clock\n");
> >>> + ret = PTR_ERR(phy->clk_pll1);
> >>> + goto err_put_clk_mod;
> >>> + }
> >>> + }
> >>> +
> >>
> >> You have a bug here. If phy_clk_num == 1, you'll still try to lookup
> >> pll-1.
> >
> > This is actually WIP patch taken from my github. This issue was fixed
> > already locally on disk. I thought Jagan will not use it until SRAM C
> > patches land.>
> >> And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
> >> lookup pll-2 either.
> >
> > It is highly unlikely this will be higher than 2, at least for this HDMI
> > PHY, since it has only 1 bit reserved for parent selection. But since I
> > have to fix it, I'll add ">= 2"
> >
> >>> + ret = sun8i_phy_clk_create(phy, dev, phy->variant->phy_clk_num);
> >>>
> >>> if (ret) {
> >>>
> >>> dev_err(dev, "Couldn't create the PHY clock\n");
> >>> goto err_put_clk_pll0;
> >>>
> >>> @@ -515,8 +530,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
> >>> struct device_node *node)>
> >>>
> >>> err_put_rst_phy:
> >>> reset_control_put(phy->rst_phy);
> >>>
> >>> err_put_clk_pll0:
> >>> - if (phy->variant->has_phy_clk)
> >>> - clk_put(phy->clk_pll0);
> >>> + clk_put(phy->clk_pll0);
> >>> + clk_put(phy->clk_pll1);
> >>>
> >>> err_put_clk_mod:
> >>> clk_put(phy->clk_mod);
> >>>
> >>> err_put_clk_bus:
> >>> @@ -536,8 +551,8 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
> >>> *hdmi)
> >>>
> >>> reset_control_put(phy->rst_phy);
> >>>
> >>> - if (phy->variant->has_phy_clk)
> >>> - clk_put(phy->clk_pll0);
> >>> + clk_put(phy->clk_pll0);
> >>> + clk_put(phy->clk_pll1);
> >>>
> >>> clk_put(phy->clk_mod);
> >>> clk_put(phy->clk_bus);
> >>>
> >>> }
> >>>
> >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> >>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c index
> >>> faea449812f8..85b12fc96dbc 100644
> >>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> >>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> >>> @@ -22,29 +22,36 @@ static int sun8i_phy_clk_determine_rate(struct
> >>> clk_hw
> >>> *hw,>
> >>>
> >>> {
> >>>
> >>> unsigned long rate = req->rate;
> >>> unsigned long best_rate = 0;
> >>>
> >>> - struct clk_hw *parent;
> >>> + struct clk_hw *best_parent = NULL;
> >>> + struct clk_hw *parent = NULL;
> >>>
> >>> int best_div = 1;
> >>>
> >>> - int i;
> >>> + int i, p;
> >>>
> >>> - parent = clk_hw_get_parent(hw);
> >>> -
> >>> - for (i = 1; i <= 16; i++) {
> >>> - unsigned long ideal = rate * i;
> >>> - unsigned long rounded;
> >>> -
> >>> - rounded = clk_hw_round_rate(parent, ideal);
> >>> -
> >>> - if (rounded == ideal) {
> >>> - best_rate = rounded;
> >>> - best_div = i;
> >>> - break;
> >>> - }
> >>> + for (p = 0; p < clk_hw_get_num_parents(hw); p++) {
> >>> + parent = clk_hw_get_parent_by_index(hw, p);
> >>> + if (!parent)
> >>> + continue;
> >>>
> >>> - if (!best_rate ||
> >>> - abs(rate - rounded / i) <
> >>> - abs(rate - best_rate / best_div)) {
> >>> - best_rate = rounded;
> >>> - best_div = i;
> >>> + for (i = 1; i <= 16; i++) {
> >>> + unsigned long ideal = rate * i;
> >>> + unsigned long rounded;
> >>> +
> >>> + rounded = clk_hw_round_rate(parent, ideal);
> >>> +
> >>> + if (rounded == ideal) {
> >>> + best_rate = rounded;
> >>> + best_div = i;
> >>> + best_parent = parent;
> >>> + break;
> >>> + }
> >>> +
> >>> + if (!best_rate ||
> >>> + abs(rate - rounded / i) <
> >>> + abs(rate - best_rate / best_div)) {
> >>> + best_rate = rounded;
> >>> + best_div = i;
> >>> + best_parent = parent;
> >>> + }
> >>>
> >>> }
> >>>
> >>> }
> >>>
> >>> @@ -95,22 +102,58 @@ static int sun8i_phy_clk_set_rate(struct clk_hw
> >>> *hw,
> >>> unsigned long rate,>
> >>>
> >>> return 0;
> >>>
> >>> }
> >>>
> >>> +static u8 sun8i_phy_clk_get_parent(struct clk_hw *hw)
> >>> +{
> >>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
> >>> + u32 reg;
> >>> +
> >>> + regmap_read(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, ®);
> >>> + reg = (reg & SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK) >>
> >>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT;
> >>> +
> >>> + return reg;
> >>> +}
> >>> +
> >>> +static int sun8i_phy_clk_set_parent(struct clk_hw *hw, u8 index)
> >>> +{
> >>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
> >>> +
> >>> + if (index > 1)
> >>> + return -EINVAL;
> >>> +
> >>> + regmap_update_bits(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> >>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> >>> + index << SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>
> >> The DT bindings changes and the clk changes should be part of separate
> >> patches.
> >
> > By DT bindings changes you mean code which reads DT and not DT
> > documentation, right?
> >
> > Ok, I'll split it.
> >
> > BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
> > there is nothing to hold it back, unlike for this.
> >
> > Best regards,
> > Jernej
> >
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> you have been talking about SRAM patches, required for A64 DE2, for
> about a half a year.
> May I ask you to explain in a couple of words why they are so important ?
> I am really curious because I have DE2 already working on my A64 without
> those magic patches..
>
You probably have HDMI enabled in U-Boot, right? If you disable that driver in
U-Boot, Linux driver shouldn't work anymore. There is consensus that Linux A64
DE2 driver shouldn't rely on U-Boot setting bits. Those SRAM C patches will
probably also affect how DT DE2 entries are written, especially if it will be
implemented as a bus, as once proposed by Icenowy.
Best regards,
Jernej
^ permalink raw reply
* [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Sergey Suloev @ 2018-05-18 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3135535.HuDyCXIRmt@jernej-laptop>
Hi, Jernej,
On 05/18/2018 06:15 PM, Jernej ?krabec wrote:
> Hi,
>
> Dne petek, 18. maj 2018 ob 17:09:40 CEST je Sergey Suloev napisal(a):
>> Hi, guys,
>>
>> On 05/18/2018 05:46 PM, Jernej ?krabec wrote:
>>> Hi,
>>>
>>> Dne petek, 18. maj 2018 ob 12:01:16 CEST je Maxime Ripard napisal(a):
>>>> On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:
>>>>> From: Jernej Skrabec <jernej.skrabec@siol.net>
>>>>>
>>>>> Some SoCs with DW HDMI have multiple possible clock parents, like A64
>>>>> and R40.
>>>>>
>>>>> Expand HDMI PHY clock driver to support second clock parent.
>>>>>
>>>>> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>>>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>>>> ---
>>>>> Changes for v2:
>>>>> - new patch
>>>>>
>>>>> drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 9 ++-
>>>>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 33 ++++++++---
>>>>> drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89
>>>>> ++++++++++++++++++++++-------- 3 files changed, 96 insertions(+), 35
>>>>> deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>>>> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..303189d6602c
>>>>> 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>>>> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
>>>>> @@ -98,7 +98,8 @@
>>>>>
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29)
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28)
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27)
>>>>>
>>>>> -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26)
>>>>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
>>>>> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT 26
>>>>>
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25)
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22)
>>>>> #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20)
>>>>>
>>>>> @@ -146,7 +147,7 @@
>>>>>
>>>>> struct sun8i_hdmi_phy;
>>>>>
>>>>> struct sun8i_hdmi_phy_variant {
>>>>>
>>>>> - bool has_phy_clk;
>>>>> + int phy_clk_num;
>>>>>
>>>>> void (*phy_init)(struct sun8i_hdmi_phy *phy);
>>>>> void (*phy_disable)(struct dw_hdmi *hdmi,
>>>>>
>>>>> struct sun8i_hdmi_phy *phy);
>>>>>
>>>>> @@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {
>>>>>
>>>>> struct clk *clk_mod;
>>>>> struct clk *clk_phy;
>>>>> struct clk *clk_pll0;
>>>>>
>>>>> + struct clk *clk_pll1;
>>>>>
>>>>> unsigned int rcal;
>>>>> struct regmap *regs;
>>>>> struct reset_control *rst_phy;
>>>>>
>>>>> @@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
>>>>> *hdmi);
>>>>>
>>>>> void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
>>>>> const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
>>>>>
>>>>> -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
>>>>> *dev);
>>>>> +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device
>>>>> *dev,
>>>>> + int clk_num);
>>>>>
>>>>> #endif /* _SUN8I_DW_HDMI_H_ */
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>>>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index
>>>>> 5a52fc489a9d..0eadf087fc46
>>>>> 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>>>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
>>>>> @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi
>>>>> *hdmi,>
>>>>>
>>>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
>>>>>
>>>>> SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
>>>>>
>>>>> - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
>>>>> + /*
>>>>> + * NOTE: We have to be careful not to overwrite PHY parent
>>>>> + * clock selection bit and clock divider.
>>>>> + */
>>>>> + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
>>>>> + (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
>>>>> + pll_cfg1_init);
>>>>>
>>>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
>>>>>
>>>>> (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
>>>>> pll_cfg2_init);
>>>>>
>>>>> @@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi
>>>>> *hdmi,
>>>>> void *data,>
>>>>>
>>>>> regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
>>>>>
>>>>> SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
>>>>>
>>>>> - if (phy->variant->has_phy_clk)
>>>>> + if (phy->variant->phy_clk_num)
>>>>>
>>>>> clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);
>>>>>
>>>>> return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);
>>>>>
>>>>> @@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant
>>>>> sun8i_a83t_hdmi_phy = {>
>>>>>
>>>>> };
>>>>>
>>>>> static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
>>>>>
>>>>> - .has_phy_clk = true,
>>>>> + .phy_clk_num = 1,
>>>>>
>>>>> .phy_init = &sun8i_hdmi_phy_init_h3,
>>>>> .phy_disable = &sun8i_hdmi_phy_disable_h3,
>>>>> .phy_config = &sun8i_hdmi_phy_config_h3,
>>>>>
>>>>> @@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
>>>>> struct device_node *node)>
>>>>>
>>>>> goto err_put_clk_bus;
>>>>>
>>>>> }
>>>>>
>>>>> - if (phy->variant->has_phy_clk) {
>>>>> + if (phy->variant->phy_clk_num) {
>>>>>
>>>>> phy->clk_pll0 = of_clk_get_by_name(node, "pll-0");
>>>>> if (IS_ERR(phy->clk_pll0)) {
>>>>>
>>>>> dev_err(dev, "Could not get pll-0 clock\n");
>>>>>
>>>>> @@ -472,7 +478,16 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi
>>>>> *hdmi,
>>>>> struct device_node *node)>
>>>>>
>>>>> goto err_put_clk_mod;
>>>>>
>>>>> }
>>>>>
>>>>> - ret = sun8i_phy_clk_create(phy, dev);
>>>>> + if (phy->variant->phy_clk_num) {
>>>>> + phy->clk_pll1 = of_clk_get_by_name(node, "pll-1");
>>>>> + if (IS_ERR(phy->clk_pll1)) {
>>>>> + dev_err(dev, "Could not get pll-1 clock\n");
>>>>> + ret = PTR_ERR(phy->clk_pll1);
>>>>> + goto err_put_clk_mod;
>>>>> + }
>>>>> + }
>>>>> +
>>>> You have a bug here. If phy_clk_num == 1, you'll still try to lookup
>>>> pll-1.
>>> This is actually WIP patch taken from my github. This issue was fixed
>>> already locally on disk. I thought Jagan will not use it until SRAM C
>>> patches land.>
>>>> And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
>>>> lookup pll-2 either.
>>> It is highly unlikely this will be higher than 2, at least for this HDMI
>>> PHY, since it has only 1 bit reserved for parent selection. But since I
>>> have to fix it, I'll add ">= 2"
>>>
>>>>> + ret = sun8i_phy_clk_create(phy, dev, phy->variant->phy_clk_num);
>>>>>
>>>>> if (ret) {
>>>>>
>>>>> dev_err(dev, "Couldn't create the PHY clock\n");
>>>>> goto err_put_clk_pll0;
>>>>>
>>>>> @@ -515,8 +530,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi,
>>>>> struct device_node *node)>
>>>>>
>>>>> err_put_rst_phy:
>>>>> reset_control_put(phy->rst_phy);
>>>>>
>>>>> err_put_clk_pll0:
>>>>> - if (phy->variant->has_phy_clk)
>>>>> - clk_put(phy->clk_pll0);
>>>>> + clk_put(phy->clk_pll0);
>>>>> + clk_put(phy->clk_pll1);
>>>>>
>>>>> err_put_clk_mod:
>>>>> clk_put(phy->clk_mod);
>>>>>
>>>>> err_put_clk_bus:
>>>>> @@ -536,8 +551,8 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi
>>>>> *hdmi)
>>>>>
>>>>> reset_control_put(phy->rst_phy);
>>>>>
>>>>> - if (phy->variant->has_phy_clk)
>>>>> - clk_put(phy->clk_pll0);
>>>>> + clk_put(phy->clk_pll0);
>>>>> + clk_put(phy->clk_pll1);
>>>>>
>>>>> clk_put(phy->clk_mod);
>>>>> clk_put(phy->clk_bus);
>>>>>
>>>>> }
>>>>>
>>>>> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>>>> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c index
>>>>> faea449812f8..85b12fc96dbc 100644
>>>>> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>>>> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
>>>>> @@ -22,29 +22,36 @@ static int sun8i_phy_clk_determine_rate(struct
>>>>> clk_hw
>>>>> *hw,>
>>>>>
>>>>> {
>>>>>
>>>>> unsigned long rate = req->rate;
>>>>> unsigned long best_rate = 0;
>>>>>
>>>>> - struct clk_hw *parent;
>>>>> + struct clk_hw *best_parent = NULL;
>>>>> + struct clk_hw *parent = NULL;
>>>>>
>>>>> int best_div = 1;
>>>>>
>>>>> - int i;
>>>>> + int i, p;
>>>>>
>>>>> - parent = clk_hw_get_parent(hw);
>>>>> -
>>>>> - for (i = 1; i <= 16; i++) {
>>>>> - unsigned long ideal = rate * i;
>>>>> - unsigned long rounded;
>>>>> -
>>>>> - rounded = clk_hw_round_rate(parent, ideal);
>>>>> -
>>>>> - if (rounded == ideal) {
>>>>> - best_rate = rounded;
>>>>> - best_div = i;
>>>>> - break;
>>>>> - }
>>>>> + for (p = 0; p < clk_hw_get_num_parents(hw); p++) {
>>>>> + parent = clk_hw_get_parent_by_index(hw, p);
>>>>> + if (!parent)
>>>>> + continue;
>>>>>
>>>>> - if (!best_rate ||
>>>>> - abs(rate - rounded / i) <
>>>>> - abs(rate - best_rate / best_div)) {
>>>>> - best_rate = rounded;
>>>>> - best_div = i;
>>>>> + for (i = 1; i <= 16; i++) {
>>>>> + unsigned long ideal = rate * i;
>>>>> + unsigned long rounded;
>>>>> +
>>>>> + rounded = clk_hw_round_rate(parent, ideal);
>>>>> +
>>>>> + if (rounded == ideal) {
>>>>> + best_rate = rounded;
>>>>> + best_div = i;
>>>>> + best_parent = parent;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (!best_rate ||
>>>>> + abs(rate - rounded / i) <
>>>>> + abs(rate - best_rate / best_div)) {
>>>>> + best_rate = rounded;
>>>>> + best_div = i;
>>>>> + best_parent = parent;
>>>>> + }
>>>>>
>>>>> }
>>>>>
>>>>> }
>>>>>
>>>>> @@ -95,22 +102,58 @@ static int sun8i_phy_clk_set_rate(struct clk_hw
>>>>> *hw,
>>>>> unsigned long rate,>
>>>>>
>>>>> return 0;
>>>>>
>>>>> }
>>>>>
>>>>> +static u8 sun8i_phy_clk_get_parent(struct clk_hw *hw)
>>>>> +{
>>>>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
>>>>> + u32 reg;
>>>>> +
>>>>> + regmap_read(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, ®);
>>>>> + reg = (reg & SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK) >>
>>>>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT;
>>>>> +
>>>>> + return reg;
>>>>> +}
>>>>> +
>>>>> +static int sun8i_phy_clk_set_parent(struct clk_hw *hw, u8 index)
>>>>> +{
>>>>> + struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
>>>>> +
>>>>> + if (index > 1)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + regmap_update_bits(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
>>>>> + SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
>>>>> + index << SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>> The DT bindings changes and the clk changes should be part of separate
>>>> patches.
>>> By DT bindings changes you mean code which reads DT and not DT
>>> documentation, right?
>>>
>>> Ok, I'll split it.
>>>
>>> BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
>>> there is nothing to hold it back, unlike for this.
>>>
>>> Best regards,
>>> Jernej
>>>
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>> you have been talking about SRAM patches, required for A64 DE2, for
>> about a half a year.
>> May I ask you to explain in a couple of words why they are so important ?
>> I am really curious because I have DE2 already working on my A64 without
>> those magic patches..
>>
> You probably have HDMI enabled in U-Boot, right? If you disable that driver in
> U-Boot, Linux driver shouldn't work anymore. There is consensus that Linux A64
> DE2 driver shouldn't rely on U-Boot setting bits. Those SRAM C patches will
> probably also affect how DT DE2 entries are written, especially if it will be
> implemented as a bus, as once proposed by Icenowy.
>
> Best regards,
> Jernej
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
thanks, got it. yes , I think U-Boot is handing this for me. And I am
not using the "bus way".
^ permalink raw reply
* [PATCH] arm64: dts: specify 1.8V EMMC capabilities for bcm958742k
From: Scott Branden @ 2018-05-18 15:21 UTC (permalink / raw)
To: linux-arm-kernel
Specify 1.8V EMMC capabilities for bcm958742k board to indicate support
for UHS mode.
Fixes: d4b4aba6be8a ("arm64: dts: Initial DTS files for Broadcom Stingray SOC")
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts b/arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts
index eb6f08c..77efa28 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts
+++ b/arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts
@@ -43,6 +43,10 @@
enet-phy-lane-swap;
};
+&sdio0 {
+ mmc-ddr-1_8v;
+};
+
&uart2 {
status = "okay";
};
--
2.5.0
^ permalink raw reply related
* [PATCH 0/2] Fix STM32 PWM capture build with COMPILE_TEST
From: Fabrice Gasnier @ 2018-05-18 15:24 UTC (permalink / raw)
To: linux-arm-kernel
Build issue has been identified when COMPILE_TEST=y and MFD_STM32_TIMERS=n:
https://lkml.org/lkml/2018/5/17/825
- First patch introduces a stub routine in mfd header file
- Sub-sequent patch solves warning in pwm-stm32 with these configs
Fabrice Gasnier (2):
mfd: stm32-timers: fix pwm-stm32 linker issue with COMPILE_TEST
pwm: stm32: initialize raw local variables
drivers/pwm/pwm-stm32.c | 2 +-
include/linux/mfd/stm32-timers.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/2] mfd: stm32-timers: fix pwm-stm32 linker issue with COMPILE_TEST
From: Fabrice Gasnier @ 2018-05-18 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526657044-14879-1-git-send-email-fabrice.gasnier@st.com>
This is seen when COMPILE_TEST=y and MFD_STM32_TIMERS=n.
drivers/pwm/pwm-stm32.o: In function 'stm32_pwm_raw_capture':
pwm-stm32.c:... undefined reference to 'stm32_timers_dma_burst_read'
Fixes: 0c6609805b63 ("mfd: stm32-timers: Add support for DMAs")
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
include/linux/mfd/stm32-timers.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
index 9da1d7e..067d146 100644
--- a/include/linux/mfd/stm32-timers.h
+++ b/include/linux/mfd/stm32-timers.h
@@ -124,8 +124,20 @@ struct stm32_timers {
struct stm32_timers_dma dma; /* Only to be used by the parent */
};
+#if IS_REACHABLE(CONFIG_MFD_STM32_TIMERS)
int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
enum stm32_timers_dmas id, u32 reg,
unsigned int num_reg, unsigned int bursts,
unsigned long tmo_ms);
+#else
+static inline int stm32_timers_dma_burst_read(struct device *dev, u32 *buf,
+ enum stm32_timers_dmas id,
+ u32 reg,
+ unsigned int num_reg,
+ unsigned int bursts,
+ unsigned long tmo_ms)
+{
+ return -ENODEV;
+}
+#endif
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] pwm: stm32: initialize raw local variables
From: Fabrice Gasnier @ 2018-05-18 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526657044-14879-1-git-send-email-fabrice.gasnier@st.com>
This removes build warning when COMPILE_TEST=y and MFD_STM32_TIMERS=n
in drivers/pwm/pwm-stm32.c. In function 'stm32_pwm_capture' 'raw_prd' and
'raw_dty' may be used uninitialized in this function
[-Wmaybe-uninitialized]
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
drivers/pwm/pwm-stm32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
index 60bfc07..09383c6 100644
--- a/drivers/pwm/pwm-stm32.c
+++ b/drivers/pwm/pwm-stm32.c
@@ -170,7 +170,7 @@ static int stm32_pwm_capture(struct pwm_chip *chip, struct pwm_device *pwm,
unsigned long long prd, div, dty;
unsigned long rate;
unsigned int psc = 0, icpsc, scale;
- u32 raw_prd, raw_dty;
+ u32 raw_prd = 0, raw_dty = 0;
int ret = 0;
mutex_lock(&priv->lock);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Maxime Ripard @ 2018-05-18 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4909574.Q3IFWM0xt6@jernej-laptop>
On Fri, May 18, 2018 at 04:46:41PM +0200, Jernej ?krabec wrote:
> > And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
> > lookup pll-2 either.
>
> It is highly unlikely this will be higher than 2, at least for this HDMI PHY,
> since it has only 1 bit reserved for parent selection. But since I have to fix
> it, I'll add ">= 2"
If we're only going to have two parents at most, ever, why don't we
had just a single other boolean. This would be less intrusive, and we
wouldn't have to check for those corner cases.
> BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
> there is nothing to hold it back, unlike for this.
Awesome, thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180518/0f026579/attachment.sig>
^ permalink raw reply
* [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Jernej Škrabec @ 2018-05-18 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518152651.lfymc3kj7npj5tww@flea>
Hi,
Dne petek, 18. maj 2018 ob 17:26:51 CEST je Maxime Ripard napisal(a):
> On Fri, May 18, 2018 at 04:46:41PM +0200, Jernej ?krabec wrote:
> > > And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
> > > lookup pll-2 either.
> >
> > It is highly unlikely this will be higher than 2, at least for this HDMI
> > PHY, since it has only 1 bit reserved for parent selection. But since I
> > have to fix it, I'll add ">= 2"
>
> If we're only going to have two parents at most, ever, why don't we
> had just a single other boolean. This would be less intrusive, and we
> wouldn't have to check for those corner cases.
That works for me too. And since it's only the code, it can always be reworked
if there is the need.
Best regards,
Jernej
>
> > BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
> > there is nothing to hold it back, unlike for this.
>
> Awesome, thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply
* [PATCH v9 07/11] arm64: kexec_file: add crash dump support
From: Rob Herring @ 2018-05-18 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0aba6388-8a73-d371-7b92-3594639eb27e@arm.com>
On Tue, May 15, 2018 at 06:12:59PM +0100, James Morse wrote:
> Hi guys,
>
> (CC: +RobH, devicetree list)
Thanks.
> On 25/04/18 07:26, AKASHI Takahiro wrote:
> > Enabling crash dump (kdump) includes
> > * prepare contents of ELF header of a core dump file, /proc/vmcore,
> > using crash_prepare_elf64_headers(), and
> > * add two device tree properties, "linux,usable-memory-range" and
> > "linux,elfcorehdr", which represent repsectively a memory range
> > to be used by crash dump kernel and the header's location
BTW, I intend to move existing parsing these out of the arch code.
Please don't add more DT handling to arch/ unless it is *really* arch
specific. I'd assume that the next arch to add kexec support will use
these bindings instead of the powerpc way.
> kexec_file_load() on arm64 needs to be able to create a prop encoded array to
> the FDT, but there doesn't appear to be a libfdt helper to do this.
>
> Akashi's code below adds fdt_setprop_range() to the arch code, and duplicates
> bits of libfdt_internal.h to do the work.
>
> How should this be done? I'm assuming this is something we need a new API in
> libfdt.h for. How do these come about, and is there an interim step we can use
> until then?
Submit patches to upstream dtc and then we can pull it in. Ahead of that
you can add it to drivers/of/fdt.c (or maybe fdt_address.c because
that's really what this is dealing with).
libfdt has only recently gained the beginnings of address handling.
>
> Thanks!
>
> James
>
> > diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> > index 37c0a9dc2e47..ec674f4d267c 100644
> > --- a/arch/arm64/kernel/machine_kexec_file.c
> > +++ b/arch/arm64/kernel/machine_kexec_file.c
> > @@ -76,6 +81,78 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> > return ret;
> > }
> >
> > +static int __init arch_kexec_file_init(void)
> > +{
> > + /* Those values are used later on loading the kernel */
> > + __dt_root_addr_cells = dt_root_addr_cells;
> > + __dt_root_size_cells = dt_root_size_cells;
I intend to make dt_root_*_cells private, so don't add another user
outside of drivers/of/.
> > +
> > + return 0;
> > +}
> > +late_initcall(arch_kexec_file_init);
> > +
> > +#define FDT_ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1))
> > +#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))
> > +
> > +static int fdt_prop_len(const char *prop_name, int len)
> > +{
> > + return (strlen(prop_name) + 1) +
> > + sizeof(struct fdt_property) +
> > + FDT_TAGALIGN(len);
> > +}
> > +
> > +static bool cells_size_fitted(unsigned long base, unsigned long size)
I can't imagine this would happen. However, when this is moved to
drivers/of/ or dtc, these need to be u64 types to work on 32-bit.
> > +{
> > + /* if *_cells >= 2, cells can hold 64-bit values anyway */
> > + if ((__dt_root_addr_cells == 1) && (base >= (1ULL << 32)))
> > + return false;
> > +
> > + if ((__dt_root_size_cells == 1) && (size >= (1ULL << 32)))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static void fill_property(void *buf, u64 val64, int cells)
> > +{
> > + u32 val32;
This should be a __be32 or fdt32 type. So should buf.
> > +
> > + if (cells == 1) {
> > + val32 = cpu_to_fdt32((u32)val64);
> > + memcpy(buf, &val32, sizeof(val32));
> > + } else {
> > + memset(buf, 0, cells * sizeof(u32) - sizeof(u64));
> > + buf += cells * sizeof(u32) - sizeof(u64);
> > +
> > + val64 = cpu_to_fdt64(val64);
> > + memcpy(buf, &val64, sizeof(val64));
Look how of_read_number() is implemented. You should be able to do
something similar here looping and avoiding the if/else.
> > + }
> > +}
> > +
> > +static int fdt_setprop_range(void *fdt, int nodeoffset, const char *name,
> > + unsigned long addr, unsigned long size)
A very generic sounding function, but really only works on addresses in
children of the root node.
> > +{
> > + void *buf, *prop;
> > + size_t buf_size;
> > + int result;
> > +
> > + buf_size = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
> > + prop = buf = vmalloc(buf_size);
This can go on the stack instead (and would be required to to work in
libfdt).
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + fill_property(prop, addr, __dt_root_addr_cells);
> > + prop += __dt_root_addr_cells * sizeof(u32);
> > +
> > + fill_property(prop, size, __dt_root_size_cells);
> > +
> > + result = fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
> > +
> > + vfree(buf);
> > +
> > + return result;
> > +}
> > +
> > static int setup_dtb(struct kimage *image,
> > unsigned long initrd_load_addr, unsigned long initrd_len,
> > char *cmdline, unsigned long cmdline_len,
> > @@ -88,10 +165,26 @@ static int setup_dtb(struct kimage *image,
> > int range_len;
> > int ret;
> >
> > + /* check ranges against root's #address-cells and #size-cells */
> > + if (image->type == KEXEC_TYPE_CRASH &&
> > + (!cells_size_fitted(image->arch.elf_load_addr,
> > + image->arch.elf_headers_sz) ||
> > + !cells_size_fitted(crashk_res.start,
> > + crashk_res.end - crashk_res.start + 1))) {
> > + pr_err("Crash memory region doesn't fit into DT's root cell sizes.\n");
> > + ret = -EINVAL;
> > + goto out_err;
> > + }
> > +
> > /* duplicate dt blob */
> > buf_size = fdt_totalsize(initial_boot_params);
> > range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
> >
> > + if (image->type == KEXEC_TYPE_CRASH)
> > + buf_size += fdt_prop_len("linux,elfcorehdr", range_len)
> > + + fdt_prop_len("linux,usable-memory-range",
> > + range_len);
> > +
> > if (initrd_load_addr)
> > buf_size += fdt_prop_len("linux,initrd-start", sizeof(u64))
> > + fdt_prop_len("linux,initrd-end", sizeof(u64));
> > @@ -113,6 +206,23 @@ static int setup_dtb(struct kimage *image,
> > if (nodeoffset < 0)
> > goto out_err;
> >
> > + if (image->type == KEXEC_TYPE_CRASH) {
> > + /* add linux,elfcorehdr */
> > + ret = fdt_setprop_range(buf, nodeoffset, "linux,elfcorehdr",
> > + image->arch.elf_load_addr,
> > + image->arch.elf_headers_sz);
> > + if (ret)
> > + goto out_err;
> > +
> > + /* add linux,usable-memory-range */
> > + ret = fdt_setprop_range(buf, nodeoffset,
> > + "linux,usable-memory-range",
> > + crashk_res.start,
> > + crashk_res.end - crashk_res.start + 1);
> > + if (ret)
> > + goto out_err;
> > + }
> > +
> > /* add bootargs */
> > if (cmdline) {
> > ret = fdt_setprop(buf, nodeoffset, "bootargs",
>
^ permalink raw reply
* [PATCH v10 25/27] ARM: davinci: add device tree support to timer
From: David Lechner @ 2018-05-18 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c7064bee-d385-4496-c19e-1b72853b01f1@ti.com>
On 05/18/2018 01:05 AM, Sekhar Nori wrote:
> On Thursday 17 May 2018 08:39 PM, David Lechner wrote:
>> On 05/17/2018 09:35 AM, Sekhar Nori wrote:
>>> Hi David,
>>>
>>> On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
>>>> This adds device tree support to the davinci timer so that when clocks
>>>> are moved to device tree, the timer will still work.
>>>>
>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>> ---
>>>
>>>> +static int __init of_davinci_timer_init(struct device_node *np)
>>>> +{
>>>> +??? struct clk *clk;
>>>> +
>>>> +??? clk = of_clk_get(np, 0);
>>>> +??? if (IS_ERR(clk)) {
>>>> +??????? struct of_phandle_args clkspec;
>>>> +
>>>> +??????? /*
>>>> +???????? * Fall back to using ref_clk if the actual clock is not
>>>> +???????? * available. There will be problems later if the real clock
>>>> +???????? * source is disabled.
>>>> +???????? */
>>>> +
>>>> +??????? pr_warn("%s: falling back to ref_clk\n", __func__);
>>>> +
>>>> +??????? clkspec.np = of_find_node_by_name(NULL, "ref_clk");
>>>> +??????? if (IS_ERR(clkspec.np)) {
>>>> +??????????? pr_err("%s: No clock available for timer!\n", __func__);
>>>> +??????????? return PTR_ERR(clkspec.np);
>>>> +??????? }
>>>> +??????? clk = of_clk_get_from_provider(&clkspec);
>>>> +??????? of_node_put(clkspec.np);
>>>> +??? }
>>>
>>> Do we need this error path now?
>>>
>>> Thanks,
>>> Sekhar
>>>
>>
>> No, not really.
>
> Then lets just print an error and return the error number.
>
OK. FYI, timer_probe() prints the error if we return and error, so
I will just return the error.
^ permalink raw reply
* [PATCH v5 4/4] drm/rockchip: support dp training outside dp firmware
From: Sean Paul @ 2018-05-18 15:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <11928313.3EhRqhFFHB@phil>
On Fri, May 18, 2018 at 10:52:17AM +0200, Heiko Stuebner wrote:
> Am Freitag, 18. Mai 2018, 03:45:46 CEST schrieb Brian Norris:
> > On Thu, May 17, 2018 at 6:41 PM, hl <hl@rock-chips.com> wrote:
> > > On Thursday, May 17, 2018 09:51 PM, Sean Paul wrote:
> > >> On Thu, May 17, 2018 at 05:18:00PM +0800, Lin Huang wrote:
> > >>> DP firmware uses fixed phy config values to do training, but some
> > >>> boards need to adjust these values to fit for their unique hardware
> > >>> design. So get phy config values from dts and use software link training
> > >>> instead of relying on firmware, if software training fail, keep firmware
> > >>> training as a fallback if sw training fails.
> > >>>
> > >>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> > >>> Signed-off-by: Lin Huang <hl@rock-chips.com>
> > >>> ---
> > >>> Changes in v2:
> > >>> - update patch following Enric suggest
> > >>> Changes in v3:
> > >>> - use variable fw_training instead sw_training_success
> > >>> - base on DP SPCE, if training fail use lower link rate to retry training
> > >>> Changes in v4:
> > >>> - improve cdn_dp_get_lower_link_rate() and cdn_dp_software_train_link() follow Sean suggest
> > >>> Changes in v5:
> > >>> - fix some whitespcae issue
> > >>>
> > >>> drivers/gpu/drm/rockchip/Makefile | 3 +-
> > >>> drivers/gpu/drm/rockchip/cdn-dp-core.c | 24 +-
> > >>> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +
> > >>> drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 420 ++++++++++++++++++++++++
> > >>> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 31 +-
> > >>> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
> > >>> 6 files changed, 505 insertions(+), 13 deletions(-)
> > >>> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> > >>>
> > ...
> > >>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> > >>> new file mode 100644
> > >>> index 0000000..73c3290
> > >>> --- /dev/null
> > >>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> > >>> @@ -0,0 +1,420 @@
> > >>> +// SPDX-License-Identifier: GPL-2.0
> > >>> +/*
> > >>> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> > >>> + * Author: Chris Zhong <zyw@rock-chips.com>
> > >>> + */
> > >>> +
> > >>> +#include <linux/device.h>
> > >>> +#include <linux/delay.h>
> > >>> +#include <linux/phy/phy.h>
> > >>> +#include <soc/rockchip/rockchip_phy_typec.h>
> > >>> +
> > >>> +#include "cdn-dp-core.h"
> > >>> +#include "cdn-dp-reg.h"
> > >>> +
> > >>> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
> > >>> +{
> > >>> + struct cdn_dp_port *port = dp->port[dp->active_port];
> > >>> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
> > >>
> > >> You ignored Brian's comment on the previous patch:
> > >> This is still antithetical to the PHY framework; you're assuming that
> > >> this is a particular type of PHY here.
> > >>
> > >> FWIW, the mediatek drm driver also assumes a certain PHY type. A quick grep of
> > >> drivers/ shows that the only other non-phy/ driver using this function
> > >> (pinctrl-tegra-xusb.c) also casts it.
> > >>
> > >> Sean
> > >
> > > Thanks Sean, except phy framework have new API to handle it, i have not
> > > idea how to do it in a better way.
> >
> > Well, if Mediatek can do it for their MIPI and HDMI, then maybe we just do it...
>
> I'd think so too. This is in Rockchip-specific code so it will always be
> possible to easily get the soc-type and thus phy-type, if that combination
> really changes down the road.
>
So in the absence of a better solution, and with prior art,
Reviewed-by: Sean Paul <seanpaul@chromium.org>
We just need some eyes on the dt and phy changes in this set. Heiko, can you
help out with that?
Sean
>
> Heiko
>
>
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply
* [PATCH 6/6] arm64: perf: Add support for chaining counters
From: Suzuki K Poulose @ 2018-05-18 15:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <32f2d870-9b9c-0b10-880f-cc99cf969bf6@arm.com>
Hi Robin,
On 18/05/18 14:49, Robin Murphy wrote:
> On 18/05/18 11:22, Suzuki K Poulose wrote:
>> Add support for chained event counters. PMUv3 allows chaining
>> a pair of adjacent PMU counters (with the lower counter number
>> being always "even"). The low counter is programmed to count
>> the event of interest and the high counter(odd numbered) is
>> programmed with a special event code (0x1e - Chain). Thus
>> we need special allocation schemes to make the full use of
>> available counters. So, we allocate the counters from either
>> ends. i.e, chained counters are allocated from the lower
>> end in pairs of two and the normal counters are allocated
>> from the higher number. Also makes necessary changes to
>> handle the chained events as a single event with 2 counters.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>> ? arch/arm64/kernel/perf_event.c | 226 ++++++++++++++++++++++++++++++++++++-----
>> ? 1 file changed, 202 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index ea8e060..5f81cd0 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -446,9 +446,11 @@ static struct attribute_group armv8_pmuv3_events_attr_group = {
..
>> +static inline u64 armv8pmu_read_chain_counter(int idx)
>> +{
>> +??? u64 prev_hi, hi, lo;
>> +
>> +??? do {
>> +??????? prev_hi = armv8pmu_read_evcntr(idx);
>> +??????? isb();
>> +??????? lo = armv8pmu_read_evcntr(idx - 1);
>> +??????? isb();
>> +??????? hi = armv8pmu_read_evcntr(idx);
>> +??????? isb();
>> +??? } while (prev_hi != hi);
>
> Is it worth trying to elide that last isb() in the highly likely case that we don't need it?
You're right. Also, I will rework the code to reuse the "hi".
>> +static inline void armv8pmu_write_chain_counter(int idx, u64 value)
>> +{
>> +??? armv8pmu_write_evcntr(idx, value >> 32);
>> +??? isb();
>> +??? armv8pmu_write_evcntr(idx - 1, value);
>> +??? isb();
>
> Either that isb() is unnecessary, or we are (and have been) missing one after a non-chained write.
Thats right, it is not necessary, will remove it.
>> ? static inline int armv8pmu_disable_counter(int idx)
>> ? {
>> ????? u32 counter = ARMV8_IDX_TO_COUNTER(idx);
>> @@ -567,6 +669,24 @@ static inline int armv8pmu_disable_counter(int idx)
>> ????? return idx;
>> ? }
>> +static inline void armv8pmu_disable_event_counter(struct perf_event *event)
>> +{
>> +??? struct hw_perf_event *hwc = &event->hw;
>
> Nit: might as well drop this and be consistent with the enable case.
Sure.
>> ? static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
>> ??????????????????? struct perf_event *event)
>> ? {
>> @@ -755,7 +915,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
>> ????? struct hw_perf_event *hwc = &event->hw;
>> ????? unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
>> -??? /* Always prefer to place a cycle counter into the cycle counter. */
>> +??? /*
>> +???? * Always prefer to place a cycle counter into the cycle counter
>> +???? * irrespective of whether we are counting 32bit/64bit
>
> I don't think that comment change adds much :/
>
Thats a left over from rebasing. Thanks for spotting.
>> ? /*
>> @@ -845,8 +1016,14 @@ static int __armv8_pmuv3_map_event(struct perf_event *event,
>> ???????????????????????? &armv8_pmuv3_perf_cache_map,
>> ???????????????????????? ARMV8_PMU_EVTYPE_EVENT);
>> -??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES)
>> +??? if (hw_event_id == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
>> +??????? /* Prevent chaining for cycle counter */
>
> Why? Sure, we want to avoid executing the chaining logic if we're scheduling a cycles event in the dedicated counter (which is perhaps what the comment above wanted to say), but if one ends up allocated into a regular counter (e.g. if the user asks for multiple cycle counts with different filters), then I don't see any reason to forbid that being chained.
Ah, I didn't think about that case. I was under the assumption that the
cycles are *only* placed on the cycle counter. I will take care of that.
Thanks for the review.
Suzuki
^ permalink raw reply
* [PATCH v9 05/11] arm64: kexec_file: load initrd and device-tree
From: James Morse @ 2018-05-18 15:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518074203.GM2737@linaro.org>
Hi Akashi,
On 18/05/18 08:42, AKASHI Takahiro wrote:
> On Fri, May 18, 2018 at 04:11:35PM +0900, AKASHI Takahiro wrote:
>> On Tue, May 15, 2018 at 05:20:00PM +0100, James Morse wrote:
>>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>>>> index f9ebf54ca247..b3b9b1725d8a 100644
>>>> --- a/arch/arm64/kernel/machine_kexec_file.c
>>>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>>>> @@ -55,3 +74,144 @@ int arch_kexec_walk_mem(struct kexec_buf *kbuf,
>>>> + buf = vmalloc(buf_size);
>>>> + if (!buf) {
>>>> + ret = -ENOMEM;
>>>> + goto out_err;
>>>> + }
>>>> +
>>>> + ret = fdt_open_into(initial_boot_params, buf, buf_size);
>>>> + if (ret)
>>>> + goto out_err;
>>>> +
>>>> + nodeoffset = fdt_path_offset(buf, "/chosen");
>>>> + if (nodeoffset < 0)
>>>> + goto out_err;
>>>> +
>>>> + /* add bootargs */
>>>> + if (cmdline) {
>>>> + ret = fdt_setprop(buf, nodeoffset, "bootargs",
>>>> + cmdline, cmdline_len + 1);
>>>
>>> fdt_setprop_string()?
>>
>> OK
>
> cmdline_len is passed by system call, kexec_file_load(), and this means
> that we can't believe that cmdline is always terminated with '\0'.
Yuck, we expect user-space to tell us how long the string is. It may be worth a
comment that it isn't necessarily null-terminated, as that is surprising!
(I assume the DT's property length is enough to make that safe for the new
kernel to read).
>>>> + /* within 1GB-aligned window of up to 32GB in size */
>>>> + kbuf.buf_max = round_down(kern_seg->mem, SZ_1G)
>>>> + + (unsigned long)SZ_1G * 32;
>>>> + kbuf.top_down = false;
>>>> +
>>>> + ret = kexec_add_buffer(&kbuf);
>>>> + if (ret)
>>>> + goto out_err;
>>>> + initrd_load_addr = kbuf.mem;
>>>> +
>>>> + pr_debug("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
>>>> + initrd_load_addr, initrd_len, initrd_len);
>>>> + }
>>>> +
>>>> + /* load dtb blob */
>>>> + ret = setup_dtb(image, initrd_load_addr, initrd_len,
>>>> + cmdline, cmdline_len, &dtb, &dtb_len);
>>>> + if (ret) {
>>>> + pr_err("Preparing for new dtb failed\n");
>>>> + goto out_err;
>>>> + }
>>>> +
>>>> + kbuf.buffer = dtb;
>>>> + kbuf.bufsz = dtb_len;
>>>> + kbuf.memsz = dtb_len;
>>>> + /* not across 2MB boundary */
>>>> + kbuf.buf_align = SZ_2M;
>>>> + kbuf.buf_max = ULONG_MAX;
>>>> + kbuf.top_down = true;
>>>> +
>>>> + ret = kexec_add_buffer(&kbuf);
>>>> + if (ret)
>>>> + goto out_err;
>>>> + image->arch.dtb_mem = kbuf.mem;
>>>> + image->arch.dtb_buf = dtb;
>>>> +
>>>> + pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
>>>> + kbuf.mem, dtb_len, dtb_len);
>>>> +
>>>> + return 0;
>>>> +
>>>> +out_err:
>>>> + vfree(dtb);
>>>> + image->arch.dtb_buf = NULL;
>>>
>>> Won't kimage_file_post_load_cleanup() always be called if we return an error
>>> here? Why not leave the free()ing until then?
>>
>> Right.
>> The reason why I left the code here was that we'd better locally clean up
>> all the stuff that were locally allocated if we trivially need to (and can)
>> do so.
>>
>> As it's redundant, I will remove it.
>
> will remove only "image->arch.dtb_buf = NULL."
Ah, because you haven't set the arch.dtb_buf pointer yet.
What about in patch 7 where you expect kimage_file_prepare_segments() to call
arch_kimage_file_post_load_cleanup() to free the arch.elf_headers? I'd expect
the free()ing to always happen in one place.
Thanks,
James
^ permalink raw reply
* [PATCH v9 07/11] arm64: kexec_file: add crash dump support
From: James Morse @ 2018-05-18 16:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518103925.GP2737@linaro.org>
Hi Akashi,
On 18/05/18 11:39, AKASHI Takahiro wrote:
> On Tue, May 15, 2018 at 06:11:15PM +0100, James Morse wrote:
>> On 25/04/18 07:26, AKASHI Takahiro wrote:
>>> Enabling crash dump (kdump) includes
>>> * prepare contents of ELF header of a core dump file, /proc/vmcore,
>>> using crash_prepare_elf64_headers(), and
>>> * add two device tree properties, "linux,usable-memory-range" and
>>> "linux,elfcorehdr", which represent repsectively a memory range
>>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>>> index 37c0a9dc2e47..ec674f4d267c 100644
>>> --- a/arch/arm64/kernel/machine_kexec_file.c
>>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>>> +static void fill_property(void *buf, u64 val64, int cells)
>>> +{
>>> + u32 val32;
>>> +
>>> + if (cells == 1) {
>>> + val32 = cpu_to_fdt32((u32)val64);
>>> + memcpy(buf, &val32, sizeof(val32));
>>> + } else {
>>
>>> + memset(buf, 0, cells * sizeof(u32) - sizeof(u64));
>>> + buf += cells * sizeof(u32) - sizeof(u64);
>>
>> Is this trying to clear the 'top' cells and shuffle the pointer to point at the
>> 'bottom' 2? I'm pretty sure this isn't endian safe.
>>
>> Do we really expect a system to have #address-cells > 2?
>
> I don't know, but just for safety.
Okay, so this is aiming to be a cover-all-cases library function.
>>> + val64 = cpu_to_fdt64(val64);
>>> + memcpy(buf, &val64, sizeof(val64));
>>> + }
>>> +}
>>> +
>>> +static int fdt_setprop_range(void *fdt, int nodeoffset, const char *name,
>>> + unsigned long addr, unsigned long size)
>>
>> (the device-tree spec describes a 'ranges' property, which had me confused. This
>> is encoding a prop-encoded-array)
>
> Should we rename it to, say, fdt_setprop_reg()?
Sure, but I'd really like this code to come from libfdt. I'm hoping for some
temporary workaround, lets see what the DT folk say.
>>> + if (!buf)
>>> + return -ENOMEM;
>>> +
>>> + fill_property(prop, addr, __dt_root_addr_cells);
>>> + prop += __dt_root_addr_cells * sizeof(u32);
>>> +
>>> + fill_property(prop, size, __dt_root_size_cells);
>>> +
>>> + result = fdt_setprop(fdt, nodeoffset, name, buf, buf_size);
>>> +
>>> + vfree(buf);
>>> +
>>> + return result;
>>> +}
>>
>> Doesn't this stuff belong in libfdt? I guess there is no 'add array element' api
>> because this the first time we've wanted to create a node with more than
>> key=fixed-size-value.
>>
>> I don't think this belongs in arch C code. Do we have a plan for getting libfdt
>> to support encoding prop-arrays? Can we put it somewhere anyone else duplicating
>> this will find it, until we can (re)move it?
>
> I will temporarily move all fdt-related stuff to a separate file, but
>
>> I have no idea how that happens... it looks like the devicetree list is the
>> place to ask.
>
> should we always sync with the original dtc/libfdt repository?
I thought so, libfdt is one of those external libraries that the kernel
consumes, like acpica. For acpica at least the rule is changes go upstream, then
get sync'd back.
>>> static int setup_dtb(struct kimage *image,
>>> unsigned long initrd_load_addr, unsigned long initrd_len,
>>> char *cmdline, unsigned long cmdline_len,
>>> @@ -88,10 +165,26 @@ static int setup_dtb(struct kimage *image,
>>> int range_len;
>>> int ret;
>>>
>>> + /* check ranges against root's #address-cells and #size-cells */
>>> + if (image->type == KEXEC_TYPE_CRASH &&
>>> + (!cells_size_fitted(image->arch.elf_load_addr,
>>> + image->arch.elf_headers_sz) ||
>>> + !cells_size_fitted(crashk_res.start,
>>> + crashk_res.end - crashk_res.start + 1))) {
>>> + pr_err("Crash memory region doesn't fit into DT's root cell sizes.\n");
>>> + ret = -EINVAL;
>>> + goto out_err;
>>> + }
>>
>> To check I've understood this properly: This can happen if the firmware provided
>> a DTB with 32bit address/size cells, but at least some of the memory requires 64
>> bit address/size cells. This could only happen on a UEFI system where the
>> firmware-DTB doesn't describe memory. ACPI-only systems would have the EFIstub DT.
>
> Probably, yes. I assumed the case where #address-cells and #size-cells
> were just missing in fdt.
Ah, that's another one. I just wanted to check we could boot on a system where
this can happen.
>>> /* duplicate dt blob */
>>> buf_size = fdt_totalsize(initial_boot_params);
>>> range_len = (__dt_root_addr_cells + __dt_root_size_cells) * sizeof(u32);
>>>
>>> + if (image->type == KEXEC_TYPE_CRASH)
>>> + buf_size += fdt_prop_len("linux,elfcorehdr", range_len)
>>> + + fdt_prop_len("linux,usable-memory-range",
>>> + range_len);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
>> Don't you need to add "linux,usable-memory-range" to the buf_size estimate?
>
> I think the code exists. See above.
Sorry, turns out I can't read!
>>> + if (ret)
>>> + goto out_err;
>>> + }
>>
>>> @@ -148,17 +258,109 @@ static int setup_dtb(struct kimage *image,
>>
>>> +static struct crash_mem *get_crash_memory_ranges(void)
>>> +{
>>> + unsigned int nr_ranges;
>>> + struct crash_mem *cmem;
>>> +
>>> + nr_ranges = 1; /* for exclusion of crashkernel region */
>>> + walk_system_ram_res(0, -1, &nr_ranges, get_nr_ranges_callback);
>>> +
>>> + cmem = vmalloc(sizeof(struct crash_mem) +
>>> + sizeof(struct crash_mem_range) * nr_ranges);
>>> + if (!cmem)
>>> + return NULL;
>>> +
>>> + cmem->max_nr_ranges = nr_ranges;
>>> + cmem->nr_ranges = 0;
>>> + walk_system_ram_res(0, -1, cmem, add_mem_range_callback);
>>> +
>>> + /* Exclude crashkernel region */
>>> + if (crash_exclude_mem_range(cmem, crashk_res.start, crashk_res.end)) {
>>> + vfree(cmem);
>>> + return NULL;
>>> + }
>>> +
>>> + return cmem;
>>> +}
>>
>> Could this function be included in prepare_elf_headers() so that the alloc() and
>> free() occur together.
>
> Or aiming that arm64 and x86 have similar-look code?
What's the advantage in things looking the same? If they are the same, it
probably shouldn't be in per-arch code. Otherwise it should be as simple as
possible, otherwise we can't spot the bugs/leaks.
But I think walking memblock here will remove all 'looks the same' properties here.
>>> +static int prepare_elf_headers(void **addr, unsigned long *sz)
>>> +{
>>> + struct crash_mem *cmem;
>>> + int ret = 0;
>>> +
>>> + cmem = get_crash_memory_ranges();
>>> + if (!cmem)
>>> + return -ENOMEM;
>>> +
>>> + ret = crash_prepare_elf64_headers(cmem, true, addr, sz);
>>> +
>>> + vfree(cmem);
>>
>>> + return ret;
>>> +}
>>
>> All this is moving memory-range information from core-code's
>> walk_system_ram_res() into core-code's struct crash_mem, and excluding
>> crashk_res, which again is accessible to the core code.
>>
>> It looks like this is duplicated in arch/x86 and arch/arm64 because arm64
>> doesn't have a second 'crashk_low_res' region, and always wants elf64, instead
>> of when IS_ENABLED(CONFIG_X86_64).
>> If we can abstract just those two, more of this could be moved to core code
>> where powerpc can make use of it if they want to support kdump with
>> kexec_file_load().
>>
>> But, its getting late for cross-architecture dependencies, lets put that on the
>> for-later list. (assuming there isn't a powerpc-kdump series out there adding a
>> third copy of this)
>
> Sure. X86 code has so many exceptional lines in the code :)
They also pass the e820 'usable-memory' map on the cmdline...
Thanks,
James
^ permalink raw reply
* [PATCH v5 2/4] Documentation: bindings: add phy_config for Rockchip USB Type-C PHY
From: Heiko Stuebner @ 2018-05-18 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526548680-2552-2-git-send-email-hl@rock-chips.com>
Am Donnerstag, 17. Mai 2018, 11:17:58 CEST schrieb Lin Huang:
> If want to do training outside DP Firmware, need phy voltage swing
> and pre_emphasis value.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - None
> Changes in v3:
> - modify property description and add this property to Example
> Change in v4:
> - None
> Change in v5:
> - None
>
> .../devicetree/bindings/phy/phy-rockchip-typec.txt | 29 +++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> index 960da7f..af298f2 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-typec.txt
> @@ -17,7 +17,8 @@ Required properties:
>
> Optional properties:
> - extcon : extcon specifier for the Power Delivery
> -
> + - rockchip,phy_config : A list of voltage swing(mv) and pre-emphasis
> + (dB) pairs.
mv -> mV I'd think.
This needs a bit more explanation please. Especially as the multiple "0 0"
lines in the examples suggest some deeper ordering requirements.
Like the 3 blocks of 4 entries as can be seen in the default config in
patch3. The binding should definitly describe what these are.
> Required nodes : a sub-node is required for each port the phy provides.
> The sub-node name is used to identify dp or usb3 port,
> and shall be the following entries:
> @@ -50,6 +51,19 @@ Example:
> <&cru SRST_P_UPHY0_TCPHY>;
> reset-names = "uphy", "uphy-pipe", "uphy-tcphy";
>
> + rockchip,phy_config =<0x2a 0x00
> + 0x1f 0x15
> + 0x14 0x22
> + 0x02 0x2b
> + 0x21 0x00
> + 0x12 0x15
> + 0x02 0x22
> + 0 0
> + 0x15 0x00
> + 0x00 0x15
> + 0 0
> + 0 0>;
I don't think they should be hex-values, as according to the doc above
these are values in mV and dB .
Heiko
^ permalink raw reply
* [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
From: Rob Herring @ 2018-05-18 16:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks4vjE5JMmLjjPnf6ffD9QYSUmSyrpO+yxADT_8WqAA3Tg@mail.gmail.com>
On Thu, May 17, 2018 at 08:59:40PM +0200, Benjamin Gaignard wrote:
> 2018-05-17 18:23 GMT+02:00 Rob Herring <robh+dt@kernel.org>:
> > On Wed, May 16, 2018 at 12:51 PM, William Breathitt Gray
> > <vilhelm.gray@gmail.com> wrote:
> >> From: Benjamin Gaignard <benjamin.gaignard@st.com>
> >
> > v6? Where's v1-v5?
> >
> >> Add bindings for STM32 Timer quadrature encoder.
> >> It is a sub-node of STM32 Timer which implement the
> >> counter part of the hardware.
> >>
> >> Cc: Rob Herring <robh+dt@kernel.org>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> >> ---
> >> .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++
> >> .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++
> >> 2 files changed, 33 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
> >> new file mode 100644
> >> index 000000000000..377728128bef
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
> >> @@ -0,0 +1,26 @@
> >> +STMicroelectronics STM32 Timer quadrature encoder
> >> +
> >> +STM32 Timer provides quadrature encoder counter mode to detect
> >
> > 'mode' does not sound like a sub-block of the timers block.
>
> quadrature encoding is one of the counting modes of this hardware
> block which is enable to count on other signals/triggers
You don't need a child node and compatible to set a mode.
> >> +angular position and direction of rotary elements,
> >> +from IN1 and IN2 input signals.
> >> +
> >> +Must be a sub-node of an STM32 Timer device tree node.
> >> +See ../mfd/stm32-timers.txt for details about the parent node.
> >> +
> >> +Required properties:
> >> +- compatible: Must be "st,stm32-timer-counter".
> >> +- pinctrl-names: Set to "default".
> >> +- pinctrl-0: List of phandles pointing to pin configuration nodes,
> >> + to set IN1/IN2 pins in mode of operation for Low-Power
> >> + Timer input on external pin.
> >> +
> >> +Example:
> >> + timers at 40010000 {
> >> + compatible = "st,stm32-timers";
> >> + ...
> >> + counter {
> >> + compatible = "st,stm32-timer-counter";
> >
> > Is there only 1? How is the counter addressed?
>
> Yes there is only one counter per hardware block.
> Counter is addressed like the two others sub-nodes and the details
> about parent mode are describe in stm32-timers.txt
> Should I add them here too ? so example will be like that:
No, you should drop the child node and add pinctrl to the parent.
Any other functions this block has that you plan on adding? Please make
bindings as complete as possible, not what you currently have drivers
for.
> timers at 40010000 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "st,stm32-timers";
> reg = <0x40010000 0x400>;
> clocks = <&rcc 0 160>;
> clock-names = "int";
> counter {
> compatible = "st,stm32-timer-counter";
> pinctrl-names = "default";
> pinctrl-0 = <&tim1_in_pins>;
> };
> };
>
> Benjamin
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 0/5]
From: Vince Weaver @ 2018-05-18 16:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518143913.26306-1-marc.zyngier@arm.com>
On Fri, 18 May 2018, Marc Zyngier wrote:
> There is also the case of people natively running 32bit kernels on
> 64bit HW and trying to upstream unspeakable hacks, hoping that the
> stars will align and that they'll win the lottery (see [1]).
I've tested these patches on a Raspberry Pi 3B running a 32-bit upstream
(4.17-rc5-git) kernel and they work.
[ 0.472906] hw perfevents: enabled with armv8_cortex_a53 PMU driver, 7 counters available
I only needed to add this to the devicetree
arm-pmu {
compatible = "arm,cortex-a53-pmu";
interrupt-parent = <&local_intc>;
interrupts = <9 IRQ_TYPE_LEVEL_HIGH>;
};
Tested-by: Vince Weaver <vincent.weaver@maine.edu>
Vince
^ permalink raw reply
* [PATCH v5 3/4] phy: rockchip-typec: support variable phy config value
From: Heiko Stuebner @ 2018-05-18 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1526548680-2552-3-git-send-email-hl@rock-chips.com>
Hi,
Am Donnerstag, 17. Mai 2018, 11:17:59 CEST schrieb Lin Huang:
> the phy config values used to fix in dp firmware, but some boards
> need change these values to do training and get the better eye diagram
> result. So support that in phy driver.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
I don't see anything obvious.
One could argue, that splitting out of the structs into the header could
be a separate patch, especially as the reason for it is not spelled out
in the commit message at all - and the reason only becomes visible when
also reading patch4.
But what is even more important is keeping Kishon as the phy-maintainer
in the loop. I've done that here, but please make sure that following
versions also get a Cc to
Kishon Vijay Abraham I <kishon@ti.com>
As the patch will probably need an Ack to get through the drm-tree.
Heiko
> ---
> Changes in v2:
> - update patch following Enric suggest
> Changes in v3:
> - delete need_software_training variable
> - add default phy config value, if dts do not define phy config value, use these value
> Changes in v4:
> - rename variable config to tcphy_default_config
> Changes in v5:
> - None
>
> drivers/phy/rockchip/phy-rockchip-typec.c | 306 ++++++++++++++++++++----------
> include/soc/rockchip/rockchip_phy_typec.h | 63 ++++++
> 2 files changed, 271 insertions(+), 98 deletions(-)
> create mode 100644 include/soc/rockchip/rockchip_phy_typec.h
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index 76a4b58..5d8692d 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -63,6 +63,7 @@
>
> #include <linux/mfd/syscon.h>
> #include <linux/phy/phy.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>
> #define CMN_SSM_BANDGAP (0x21 << 2)
> #define CMN_SSM_BIAS (0x22 << 2)
> @@ -323,21 +324,29 @@
> * clock 0: PLL 0 div 1
> * clock 1: PLL 1 div 2
> */
> -#define CLK_PLL_CONFIG 0X30
> +#define CLK_PLL1_DIV1 0x20
> +#define CLK_PLL1_DIV2 0x30
> #define CLK_PLL_MASK 0x33
>
> #define CMN_READY BIT(0)
>
> +#define DP_PLL_CLOCK_ENABLE_ACK BIT(3)
> #define DP_PLL_CLOCK_ENABLE BIT(2)
> +#define DP_PLL_ENABLE_ACK BIT(1)
> #define DP_PLL_ENABLE BIT(0)
> #define DP_PLL_DATA_RATE_RBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR ((2 << 12) | (4 << 8))
> #define DP_PLL_DATA_RATE_HBR2 ((1 << 12) | (2 << 8))
> +#define DP_PLL_DATA_RATE_MASK 0xff00
>
> -#define DP_MODE_A0 BIT(4)
> -#define DP_MODE_A2 BIT(6)
> -#define DP_MODE_ENTER_A0 0xc101
> -#define DP_MODE_ENTER_A2 0xc104
> +#define DP_MODE_MASK 0xf
> +#define DP_MODE_ENTER_A0 BIT(0)
> +#define DP_MODE_ENTER_A2 BIT(2)
> +#define DP_MODE_ENTER_A3 BIT(3)
> +#define DP_MODE_A0_ACK BIT(4)
> +#define DP_MODE_A2_ACK BIT(6)
> +#define DP_MODE_A3_ACK BIT(7)
> +#define DP_LINK_RESET_DEASSERTED BIT(8)
>
> #define PHY_MODE_SET_TIMEOUT 100000
>
> @@ -349,51 +358,7 @@
> #define MODE_DFP_USB BIT(1)
> #define MODE_DFP_DP BIT(2)
>
> -struct usb3phy_reg {
> - u32 offset;
> - u32 enable_bit;
> - u32 write_enable;
> -};
> -
> -/**
> - * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
> - * @reg: the base address for usb3-phy config.
> - * @typec_conn_dir: the register of type-c connector direction.
> - * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
> - * @external_psm: the register of type-c phy external psm clock.
> - * @pipe_status: the register of type-c phy pipe status.
> - * @usb3_host_disable: the register of type-c usb3 host disable.
> - * @usb3_host_port: the register of type-c usb3 host port.
> - * @uphy_dp_sel: the register of type-c phy DP select control.
> - */
> -struct rockchip_usb3phy_port_cfg {
> - unsigned int reg;
> - struct usb3phy_reg typec_conn_dir;
> - struct usb3phy_reg usb3tousb2_en;
> - struct usb3phy_reg external_psm;
> - struct usb3phy_reg pipe_status;
> - struct usb3phy_reg usb3_host_disable;
> - struct usb3phy_reg usb3_host_port;
> - struct usb3phy_reg uphy_dp_sel;
> -};
> -
> -struct rockchip_typec_phy {
> - struct device *dev;
> - void __iomem *base;
> - struct extcon_dev *extcon;
> - struct regmap *grf_regs;
> - struct clk *clk_core;
> - struct clk *clk_ref;
> - struct reset_control *uphy_rst;
> - struct reset_control *pipe_rst;
> - struct reset_control *tcphy_rst;
> - const struct rockchip_usb3phy_port_cfg *port_cfgs;
> - /* mutex to protect access to individual PHYs */
> - struct mutex lock;
> -
> - bool flip;
> - u8 mode;
> -};
> +#define DP_DEFAULT_RATE 162000
>
> struct phy_reg {
> u16 value;
> @@ -417,15 +382,15 @@ struct phy_reg usb3_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL0_LF_PROG },
> };
>
> -struct phy_reg dp_pll_cfg[] = {
> +struct phy_reg dp_pll_rbr_cfg[] = {
> { 0xf0, CMN_PLL1_VCOCAL_INIT },
> { 0x18, CMN_PLL1_VCOCAL_ITER },
> { 0x30b9, CMN_PLL1_VCOCAL_START },
> - { 0x21c, CMN_PLL1_INTDIV },
> + { 0x87, CMN_PLL1_INTDIV },
> { 0, CMN_PLL1_FRACDIV },
> - { 0x5, CMN_PLL1_HIGH_THR },
> - { 0x35, CMN_PLL1_SS_CTRL1 },
> - { 0x7f1e, CMN_PLL1_SS_CTRL2 },
> + { 0x22, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> { 0x20, CMN_PLL1_DSM_DIAG },
> { 0, CMN_PLLSM1_USER_DEF_CTRL },
> { 0, CMN_DIAG_PLL1_OVRD },
> @@ -436,9 +401,52 @@ struct phy_reg dp_pll_cfg[] = {
> { 0x8, CMN_DIAG_PLL1_LF_PROG },
> { 0x100, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> { 0x7, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> - { 0x4, CMN_DIAG_PLL1_INCLK_CTRL },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> +
> +struct phy_reg dp_pll_hbr_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> };
>
> +struct phy_reg dp_pll_hbr2_cfg[] = {
> + { 0xf0, CMN_PLL1_VCOCAL_INIT },
> + { 0x18, CMN_PLL1_VCOCAL_ITER },
> + { 0x30b4, CMN_PLL1_VCOCAL_START },
> + { 0xe1, CMN_PLL1_INTDIV },
> + { 0, CMN_PLL1_FRACDIV },
> + { 0x5, CMN_PLL1_HIGH_THR },
> + { 0x8000, CMN_PLL1_SS_CTRL1 },
> + { 0, CMN_PLL1_SS_CTRL2 },
> + { 0x20, CMN_PLL1_DSM_DIAG },
> + { 0x1000, CMN_PLLSM1_USER_DEF_CTRL },
> + { 0, CMN_DIAG_PLL1_OVRD },
> + { 0, CMN_DIAG_PLL1_FBH_OVRD },
> + { 0, CMN_DIAG_PLL1_FBL_OVRD },
> + { 0x7, CMN_DIAG_PLL1_V2I_TUNE },
> + { 0x45, CMN_DIAG_PLL1_CP_TUNE },
> + { 0x8, CMN_DIAG_PLL1_LF_PROG },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE1 },
> + { 0x1, CMN_DIAG_PLL1_PTATIS_TUNE2 },
> + { 0x1, CMN_DIAG_PLL1_INCLK_CTRL },
> +};
> static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> {
> .reg = 0xff7c0000,
> @@ -463,6 +471,24 @@ static const struct rockchip_usb3phy_port_cfg rk3399_usb3phy_port_cfgs[] = {
> { /* sentinel */ }
> };
>
> +/* default phy config */
> +static const struct phy_config tcphy_default_config[3][4] = {
> + {{ .swing = 0x2a, .pe = 0x00 },
> + { .swing = 0x1f, .pe = 0x15 },
> + { .swing = 0x14, .pe = 0x22 },
> + { .swing = 0x02, .pe = 0x2b } },
> +
> + {{ .swing = 0x21, .pe = 0x00 },
> + { .swing = 0x12, .pe = 0x15 },
> + { .swing = 0x02, .pe = 0x22 },
> + { .swing = 0, .pe = 0 } },
> +
> + {{ .swing = 0x15, .pe = 0x00 },
> + { .swing = 0x00, .pe = 0x15 },
> + { .swing = 0, .pe = 0 },
> + { .swing = 0, .pe = 0 } },
> +};
> +
> static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
> {
> u32 i, rdata;
> @@ -484,7 +510,7 @@ static void tcphy_cfg_24m(struct rockchip_typec_phy *tcphy)
>
> rdata = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> rdata &= ~CLK_PLL_MASK;
> - rdata |= CLK_PLL_CONFIG;
> + rdata |= CLK_PLL1_DIV2;
> writel(rdata, tcphy->base + CMN_DIAG_HSCLK_SEL);
> }
>
> @@ -498,17 +524,44 @@ static void tcphy_cfg_usb3_pll(struct rockchip_typec_phy *tcphy)
> tcphy->base + usb3_pll_cfg[i].addr);
> }
>
> -static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy)
> +static void tcphy_cfg_dp_pll(struct rockchip_typec_phy *tcphy, int link_rate)
> {
> - u32 i;
> + struct phy_reg *phy_cfg;
> + u32 clk_ctrl;
> + u32 i, cfg_size, hsclk_sel;
> +
> + hsclk_sel = readl(tcphy->base + CMN_DIAG_HSCLK_SEL);
> + hsclk_sel &= ~CLK_PLL_MASK;
> +
> + switch (link_rate) {
> + case 162000:
> + clk_ctrl = DP_PLL_DATA_RATE_RBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_rbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_rbr_cfg);
> + break;
> + case 270000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR;
> + hsclk_sel |= CLK_PLL1_DIV2;
> + phy_cfg = dp_pll_hbr_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr_cfg);
> + break;
> + case 540000:
> + clk_ctrl = DP_PLL_DATA_RATE_HBR2;
> + hsclk_sel |= CLK_PLL1_DIV1;
> + phy_cfg = dp_pll_hbr2_cfg;
> + cfg_size = ARRAY_SIZE(dp_pll_hbr2_cfg);
> + break;
> + }
> +
> + clk_ctrl |= DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE;
> + writel(clk_ctrl, tcphy->base + DP_CLK_CTL);
>
> - /* set the default mode to RBR */
> - writel(DP_PLL_CLOCK_ENABLE | DP_PLL_ENABLE | DP_PLL_DATA_RATE_RBR,
> - tcphy->base + DP_CLK_CTL);
> + writel(hsclk_sel, tcphy->base + CMN_DIAG_HSCLK_SEL);
>
> /* load the configuration of PLL1 */
> - for (i = 0; i < ARRAY_SIZE(dp_pll_cfg); i++)
> - writel(dp_pll_cfg[i].value, tcphy->base + dp_pll_cfg[i].addr);
> + for (i = 0; i < cfg_size; i++)
> + writel(phy_cfg[i].value, tcphy->base + phy_cfg[i].addr);
> }
>
> static void tcphy_tx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> @@ -535,9 +588,10 @@ static void tcphy_rx_usb3_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0xfb, tcphy->base + XCVR_DIAG_BIDI_CTRL(lane));
> }
>
> -static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> +static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, int link_rate,
> + u8 swing, u8 pre_emp, u32 lane)
> {
> - u16 rdata;
> + u16 val;
>
> writel(0xbefc, tcphy->base + XCVR_PSM_RCTRL(lane));
> writel(0x6799, tcphy->base + TX_PSC_A0(lane));
> @@ -545,25 +599,31 @@ static void tcphy_dp_cfg_lane(struct rockchip_typec_phy *tcphy, u32 lane)
> writel(0x98, tcphy->base + TX_PSC_A2(lane));
> writel(0x98, tcphy->base + TX_PSC_A3(lane));
>
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_001(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_010(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_011(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_100(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_101(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_110(lane));
> - writel(0, tcphy->base + TX_TXCC_MGNFS_MULT_111(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_10(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_01(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> - writel(0, tcphy->base + TX_TXCC_CPOST_MULT_11(lane));
> -
> - writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> - writel(0x400, tcphy->base + TX_DIAG_TX_DRV(lane));
> -
> - rdata = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> - rdata = (rdata & 0x8fff) | 0x6000;
> - writel(rdata, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + writel(tcphy->config[swing][pre_emp].swing,
> + tcphy->base + TX_TXCC_MGNFS_MULT_000(lane));
> + writel(tcphy->config[swing][pre_emp].pe,
> + tcphy->base + TX_TXCC_CPOST_MULT_00(lane));
> +
> + if (swing == 2 && pre_emp == 0 && link_rate != 540000) {
> + writel(0x700, tcphy->base + TX_DIAG_TX_DRV(lane));
> + writel(0x13c, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + } else {
> + writel(0x128, tcphy->base + TX_TXCC_CAL_SCLR_MULT(lane));
> + writel(0x0400, tcphy->base + TX_DIAG_TX_DRV(lane));
> + }
> +
> + val = readl(tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> + val = val & 0x8fff;
> + switch (link_rate) {
> + case 162000:
> + case 270000:
> + val |= (6 << 12);
> + break;
> + case 540000:
> + val |= (4 << 12);
> + break;
> + }
> + writel(val, tcphy->base + XCVR_DIAG_PLLDRC_CTRL(lane));
> }
>
> static inline int property_enable(struct rockchip_typec_phy *tcphy,
> @@ -754,30 +814,33 @@ static int tcphy_phy_init(struct rockchip_typec_phy *tcphy, u8 mode)
> tcphy_cfg_24m(tcphy);
>
> if (mode == MODE_DFP_DP) {
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> for (i = 0; i < 4; i++)
> - tcphy_dp_cfg_lane(tcphy, i);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, i);
>
> writel(PIN_ASSIGN_C_E, tcphy->base + PMA_LANE_CFG);
> } else {
> tcphy_cfg_usb3_pll(tcphy);
> - tcphy_cfg_dp_pll(tcphy);
> + tcphy_cfg_dp_pll(tcphy, DP_DEFAULT_RATE);
> if (tcphy->flip) {
> tcphy_tx_usb3_cfg_lane(tcphy, 3);
> tcphy_rx_usb3_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 0);
> - tcphy_dp_cfg_lane(tcphy, 1);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 0);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 1);
> } else {
> tcphy_tx_usb3_cfg_lane(tcphy, 0);
> tcphy_rx_usb3_cfg_lane(tcphy, 1);
> - tcphy_dp_cfg_lane(tcphy, 2);
> - tcphy_dp_cfg_lane(tcphy, 3);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 2);
> + tcphy_dp_cfg_lane(tcphy, DP_DEFAULT_RATE, 0, 0, 3);
> }
>
> writel(PIN_ASSIGN_D_F, tcphy->base + PMA_LANE_CFG);
> }
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2 | DP_LINK_RESET_DEASSERTED;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> reset_control_deassert(tcphy->uphy_rst);
>
> @@ -990,7 +1053,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> property_enable(tcphy, &cfg->uphy_dp_sel, 1);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A2, 1000,
> + val, val & DP_MODE_A2_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> dev_err(tcphy->dev, "failed to wait TCPHY enter A2\n");
> @@ -999,13 +1062,19 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
>
> tcphy_dp_aux_calibration(tcphy);
>
> - writel(DP_MODE_ENTER_A0, tcphy->base + DP_MODE_CTL);
> + /* enter A0 mode */
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A0;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> ret = readx_poll_timeout(readl, tcphy->base + DP_MODE_CTL,
> - val, val & DP_MODE_A0, 1000,
> + val, val & DP_MODE_A0_ACK, 1000,
> PHY_MODE_SET_TIMEOUT);
> if (ret < 0) {
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
> dev_err(tcphy->dev, "failed to wait TCPHY enter A0\n");
> goto power_on_finish;
> }
> @@ -1023,6 +1092,7 @@ static int rockchip_dp_phy_power_on(struct phy *phy)
> static int rockchip_dp_phy_power_off(struct phy *phy)
> {
> struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u32 val;
>
> mutex_lock(&tcphy->lock);
>
> @@ -1031,7 +1101,10 @@ static int rockchip_dp_phy_power_off(struct phy *phy)
>
> tcphy->mode &= ~MODE_DFP_DP;
>
> - writel(DP_MODE_ENTER_A2, tcphy->base + DP_MODE_CTL);
> + val = readl(tcphy->base + DP_MODE_CTL);
> + val &= ~DP_MODE_MASK;
> + val |= DP_MODE_ENTER_A2;
> + writel(val, tcphy->base + DP_MODE_CTL);
>
> if (tcphy->mode == MODE_DISCONNECT)
> tcphy_phy_deinit(tcphy);
> @@ -1047,9 +1120,35 @@ static const struct phy_ops rockchip_dp_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static int typec_dp_phy_config(struct phy *phy, int link_rate,
> + int lanes, u8 swing, u8 pre_emp)
> +{
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy);
> + u8 i;
> +
> + tcphy_cfg_dp_pll(tcphy, link_rate);
> +
> + if (tcphy->mode == MODE_DFP_DP) {
> + for (i = 0; i < 4; i++)
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, i);
> + } else {
> + if (tcphy->flip) {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 0);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 1);
> + } else {
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 2);
> + tcphy_dp_cfg_lane(tcphy, link_rate, swing, pre_emp, 3);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> struct device *dev)
> {
> + int ret;
> +
> tcphy->grf_regs = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> if (IS_ERR(tcphy->grf_regs)) {
> @@ -1087,6 +1186,16 @@ static int tcphy_parse_dt(struct rockchip_typec_phy *tcphy,
> return PTR_ERR(tcphy->tcphy_rst);
> }
>
> + /*
> + * check if phy_config pass from dts, if no,
> + * use default phy config value.
> + */
> + ret = of_property_read_u32_array(dev->of_node, "rockchip,phy_config",
> + (u32 *)tcphy->config, sizeof(tcphy->config) / sizeof(u32));
> + if (ret)
> + memcpy(tcphy->config, tcphy_default_config,
> + sizeof(tcphy->config));
> +
> return 0;
> }
>
> @@ -1171,6 +1280,7 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
> }
> }
>
> + tcphy->typec_phy_config = typec_dp_phy_config;
> pm_runtime_enable(dev);
>
> for_each_available_child_of_node(np, child_np) {
> diff --git a/include/soc/rockchip/rockchip_phy_typec.h b/include/soc/rockchip/rockchip_phy_typec.h
> new file mode 100644
> index 0000000..be6af0e
> --- /dev/null
> +++ b/include/soc/rockchip/rockchip_phy_typec.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#ifndef __SOC_ROCKCHIP_PHY_TYPEC_H
> +#define __SOC_ROCKCHIP_PHY_TYPEC_H
> +
> +struct usb3phy_reg {
> + u32 offset;
> + u32 enable_bit;
> + u32 write_enable;
> +};
> +
> +/**
> + * struct rockchip_usb3phy_port_cfg: usb3-phy port configuration.
> + * @reg: the base address for usb3-phy config.
> + * @typec_conn_dir: the register of type-c connector direction.
> + * @usb3tousb2_en: the register of type-c force usb2 to usb2 enable.
> + * @external_psm: the register of type-c phy external psm clock.
> + * @pipe_status: the register of type-c phy pipe status.
> + * @usb3_host_disable: the register of type-c usb3 host disable.
> + * @usb3_host_port: the register of type-c usb3 host port.
> + * @uphy_dp_sel: the register of type-c phy DP select control.
> + */
> +struct rockchip_usb3phy_port_cfg {
> + unsigned int reg;
> + struct usb3phy_reg typec_conn_dir;
> + struct usb3phy_reg usb3tousb2_en;
> + struct usb3phy_reg external_psm;
> + struct usb3phy_reg pipe_status;
> + struct usb3phy_reg usb3_host_disable;
> + struct usb3phy_reg usb3_host_port;
> + struct usb3phy_reg uphy_dp_sel;
> +};
> +
> +struct phy_config {
> + int swing;
> + int pe;
> +};
> +
> +struct rockchip_typec_phy {
> + struct device *dev;
> + void __iomem *base;
> + struct extcon_dev *extcon;
> + struct regmap *grf_regs;
> + struct clk *clk_core;
> + struct clk *clk_ref;
> + struct reset_control *uphy_rst;
> + struct reset_control *pipe_rst;
> + struct reset_control *tcphy_rst;
> + const struct rockchip_usb3phy_port_cfg *port_cfgs;
> + /* mutex to protect access to individual PHYs */
> + struct mutex lock;
> + struct phy_config config[3][4];
> + bool flip;
> + u8 mode;
> + int (*typec_phy_config)(struct phy *phy, int link_rate,
> + int lanes, u8 swing, u8 pre_emp);
> +};
> +
> +#endif
>
^ permalink raw reply
* [PATCH v6 8/9] counter: stm32-lptimer: add counter device
From: Rob Herring @ 2018-05-18 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <726de278f09aa4438b97b147cd26a03009522997.1526487615.git.vilhelm.gray@gmail.com>
On Wed, May 16, 2018 at 01:52:27PM -0400, William Breathitt Gray wrote:
> From: Fabrice Gasnier <fabrice.gasnier@st.com>
>
> Add support for new counter device to stm32-lptimer.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> .../{iio => }/counter/stm32-lptimer-cnt.txt | 0
> .../devicetree/bindings/mfd/stm32-lptimer.txt | 2 +-
Please split bindings to separate patch.
> drivers/counter/Kconfig | 10 +
> drivers/counter/Makefile | 1 +
> drivers/counter/stm32-lptimer-cnt.c | 722 ++++++++++++++++++
> drivers/iio/counter/Kconfig | 9 -
> drivers/iio/counter/Makefile | 2 -
> drivers/iio/counter/stm32-lptimer-cnt.c | 382 ---------
> 8 files changed, 734 insertions(+), 394 deletions(-)
> rename Documentation/devicetree/bindings/{iio => }/counter/stm32-lptimer-cnt.txt (100%)
> create mode 100644 drivers/counter/stm32-lptimer-cnt.c
> delete mode 100644 drivers/iio/counter/stm32-lptimer-cnt.c
^ 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