* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 19:00 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <550de191-f982-4544-6fbc-bf16dfeae2c6@nvidia.com>
01.08.2019 20:58, Sowjanya Komatineni пишет:
>
> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>
>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch implements save and restore context for peripheral fixed
>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>> peripheral clock ops.
>>>>>
>>>>> During system suspend, core power goes off and looses the settings
>>>>> of the Tegra CAR controller registers.
>>>>>
>>>>> So during suspend entry clock and reset state of peripherals is saved
>>>>> and on resume they are restored to have clocks back to same rate and
>>>>> state as before suspend.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>> ++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>> +++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>> +++++++++++++++++++++++++++
>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>> 5 files changed, 138 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>> index c088e7a280df..21b24530fa00 100644
>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>> clk_hw *hw,
>>>>> return (unsigned long)rate;
>>>>> }
>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>> to_tegra_clk_periph_fixed(hw);
>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>> +
>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>> fixed->regs->enb_reg) &
>>>>> + mask;
>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>> fixed->regs->rst_reg) &
>>>>> + mask;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>> to_tegra_clk_periph_fixed(hw);
>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>> +
>>>>> + if (fixed->enb_ctx)
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_set_reg);
>>>>> + else
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_clr_reg);
>>>>> +
>>>>> + udelay(2);
>>>>> +
>>>>> + if (!fixed->rst_ctx) {
>>>>> + udelay(5); /* reset propogation delay */
>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>> };
>>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>> #define read_rst(gate) \
>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>> +#define write_rst_set(val, gate) \
>>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>>> #define write_rst_clr(val, gate) \
>>>>> writel_relaxed(val, gate->clk_base + (gate->regs->rst_clr_reg))
>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>> clk_hw *hw)
>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>> }
>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>> +
>>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>> +
>>>>> + if (gate->clk_state_ctx)
>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>> + else
>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>> +
>>>>> + udelay(5);
>>>>> +
>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>> + if (gate->rst_state_ctx)
>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>> + else
>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>> .is_enabled = clk_periph_is_enabled,
>>>>> .enable = clk_periph_enable,
>>>>> .disable = clk_periph_disable,
>>>>> + .save_context = clk_periph_gate_save_context,
>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>> };
>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>> index 58437da25156..06fb62955768 100644
>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>>> gate_ops->disable(gate_hw);
>>>>> }
>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>> +
>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>> + gate_ops->save_context(gate_hw);
>>>>> +
>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>> +{
>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>> +
>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>> +
>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>> + div_ops->restore_context(div_hw);
>>>> Could you please point to where the divider's save_context() happens?
>>>> Because I can't see it.
>>> Ah, I now see that there is no need to save the dividers context because
>>> clk itself has enough info that is needed for the context's restoring
>>> (like I pointed in the review to v6).
>>>
>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>> generic helper to get the index instead of storing it manually.
>>
>> clk_periph_get_parent basically invokes existing clk_mux_ops
>> get_parent() which is then saved in tegra_clk_periph.
>>
>> All existing drivers are using directly get_parent() from clk_mux
>> which actually gets index from the register read.
>>
>> To have this more generic w.r.t save/restore context point of view,
>> probably instead of implementing new get_parent_index helper, I think
>> its better to implement save_context and restore_context to
>> clk_mux_ops along with creating parent_index field into clk_mux to
>> cache index during set_parent.
>>
>> So we just need to invoke mux_ops save_context and restore_context.
>>
> I hope its ok to add save/restore context to clk_mux_ops to be more
> generic w.r.t save/restore context rather than get_parent_index API.
> Please confirm if you agree.
Sounds like a good idea. I see that there is a 'restoring' helper for
the generic clk_gate, seems something similar could be done for the
clk_mux. And looks like anyway you'll need to associate the parent clock
with the hw index in order to restore the muxing.
^ permalink raw reply
* [PATCH v4 0/4] net: phy: realtek: Enable configuration of RTL8211E LEDs
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
The Realtek RTL8211E allows customization of the PHY LED behavior,
like which LEDs are on for certain link speeds and which LEDs blink
when there is traffic. By default EEE LED mode is enabled, in which
a blinking LED is on for 400ms and off for 2s. This series adds a
generic device tree binding for configuring PHY LEDs and adds LED
configuration support for the RTL8211E PHY.
Certain registers of the RTL8211E can only be accessed through
a vendor specific extended page mechanism. Extended pages need
to be accessed for the LED configuration. This series adds helpers
to facilitate accessing extended pages.
Matthias Kaehlcke (4):
dt-bindings: net: phy: Add subnode for LED configuration
net: phy: Add function to retrieve LED configuration from the DT
net: phy: realtek: Add helpers for accessing RTL8211E extension pages
net: phy: realtek: configure RTL8211E LEDs
.../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++
drivers/net/phy/phy_device.c | 50 ++++++
drivers/net/phy/realtek.c | 169 ++++++++++++++++--
include/linux/phy.h | 15 ++
4 files changed, 266 insertions(+), 15 deletions(-)
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply
* [PATCH v4 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190801190759.28201-1-mka@chromium.org>
The LED behavior of some Ethernet PHYs is configurable. Add an
optional 'leds' subnode with a child node for each LED to be
configured. The binding aims to be compatible with the common
LED binding (see devicetree/bindings/leds/common.txt).
A LED can be configured to be 'on' when a link with a certain speed
is active, or to blink on RX/TX activity. For the configuration to
be effective it needs to be supported by the hardware and the
corresponding PHY driver.
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v4:
- patch added to the series
---
.../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index f70f18ff821f..81c5aacc89a5 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -153,6 +153,38 @@ properties:
Delay after the reset was deasserted in microseconds. If
this property is missing the delay will be skipped.
+patternProperties:
+ "^leds$":
+ type: object
+ description:
+ Subnode with configuration of the PHY LEDs.
+
+ patternProperties:
+ "^led@[0-9]+$":
+ type: object
+ description:
+ Subnode with the configuration of a single PHY LED.
+
+ properties:
+ reg:
+ description:
+ The ID number of the LED, typically corresponds to a hardware ID.
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+
+ linux,default-trigger:
+ description:
+ This parameter, if present, is a string specifying the trigger
+ assigned to the LED. Supported triggers are:
+ "phy_link_10m_active" - LED will be on when a 10Mb/s link is active
+ "phy_link_100m_active" - LED will be on when a 100Mb/s link is active
+ "phy_link_1g_active" - LED will be on when a 1Gb/s link is active
+ "phy_link_10g_active" - LED will be on when a 10Gb/s link is active
+ "phy_activity" - LED will blink when data is received or transmitted
+ $ref: "/schemas/types.yaml#/definitions/string"
+
+ required:
+ - reg
+
required:
- reg
@@ -173,5 +205,20 @@ examples:
reset-gpios = <&gpio1 4 1>;
reset-assert-us = <1000>;
reset-deassert-us = <2000>;
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ led@0 {
+ reg = <0>;
+ linux,default-trigger = "phy_link_1g_active";
+ };
+
+ led@1 {
+ reg = <1>;
+ linux,default-trigger = "phy_activity";
+ };
+ };
};
};
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH v4 2/4] net: phy: Add function to retrieve LED configuration from the DT
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190801190759.28201-1-mka@chromium.org>
Add a phylib function for retrieving PHY LED configuration that
is specified in the device tree using the generic binding. LEDs
can be configured to be 'on' for a certain link speed or to blink
when there is TX/RX activity.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v4:
- patch added to the series
---
drivers/net/phy/phy_device.c | 50 ++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 15 +++++++++++
2 files changed, 65 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6b5cb87f3866..b4b48de45712 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2188,6 +2188,56 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
return phydrv->config_intr && phydrv->ack_interrupt;
}
+int of_get_phy_led_cfg(struct phy_device *phydev, int led,
+ struct phy_led_config *cfg)
+{
+ struct device_node *np, *child;
+ const char *trigger;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_OF_MDIO))
+ return -ENOENT;
+
+ np = of_find_node_by_name(phydev->mdio.dev.of_node, "leds");
+ if (!np)
+ return -ENOENT;
+
+ for_each_child_of_node(np, child) {
+ u32 val;
+
+ if (!of_property_read_u32(child, "reg", &val)) {
+ if (val == (u32)led)
+ break;
+ }
+ }
+
+ if (!child)
+ return -ENOENT;
+
+ ret = of_property_read_string(child, "linux,default-trigger",
+ &trigger);
+ if (ret)
+ return ret;
+
+ if (!strcmp(trigger, "phy_link_10m_active")) {
+ cfg->trigger = PHY_LED_LINK_10M;
+ } else if (!strcmp(trigger, "phy_link_100m_active")) {
+ cfg->trigger = PHY_LED_LINK_100M;
+ } else if (!strcmp(trigger, "phy_link_1g_active")) {
+ cfg->trigger = PHY_LED_LINK_1G;
+ } else if (!strcmp(trigger, "phy_link_10g_active")) {
+ cfg->trigger = PHY_LED_LINK_10G;
+ } else if (!strcmp(trigger, "phy_activity")) {
+ cfg->trigger = PHY_LED_ACTIVITY;
+ } else {
+ phydev_warn(phydev, "trigger '%s' for LED%d is invalid\n",
+ trigger, led);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 462b90b73f93..b4693415be31 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1176,6 +1176,21 @@ int phy_ethtool_set_link_ksettings(struct net_device *ndev,
const struct ethtool_link_ksettings *cmd);
int phy_ethtool_nway_reset(struct net_device *ndev);
+enum phy_led_trigger {
+ PHY_LED_LINK_10M,
+ PHY_LED_LINK_100M,
+ PHY_LED_LINK_1G,
+ PHY_LED_LINK_10G,
+ PHY_LED_ACTIVITY,
+};
+
+struct phy_led_config {
+ enum phy_led_trigger trigger;
+};
+
+int of_get_phy_led_cfg(struct phy_device *phydev, int led,
+ struct phy_led_config *cfg);
+
#if IS_ENABLED(CONFIG_PHYLIB)
int __init mdio_bus_init(void);
void mdio_bus_exit(void);
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH v4 3/4] net: phy: realtek: Add helpers for accessing RTL8211E extension pages
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190801190759.28201-1-mka@chromium.org>
The RTL8211E has extension pages, which can be accessed after
selecting a page through a custom method. Add a function to
modify bits in a register of an extension page and a helper for
selecting an ext page. Use rtl8211e_modify_ext_paged() in
rtl8211e_config_init() instead of doing things 'manually'.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v4:
- don't add constant RTL8211E_EXT_PAGE, it's only used once,
use a literal instead
- pass 'oldpage' to phy_restore_page() in rtl8211e_select_ext_page(),
not 'page'
- return 'oldpage' in rtl8211e_select_ext_page()
- use __phy_modify() in rtl8211e_modify_ext_paged() instead of
reimplementing __phy_modify_changed()
- in rtl8211e_modify_ext_paged() return directly when
rtl8211e_select_ext_page() fails
---
drivers/net/phy/realtek.c | 48 +++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index a669945eb829..e09d3b0da2c7 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -53,6 +53,36 @@ static int rtl821x_write_page(struct phy_device *phydev, int page)
return __phy_write(phydev, RTL821x_PAGE_SELECT, page);
}
+static int rtl8211e_select_ext_page(struct phy_device *phydev, int page)
+{
+ int ret, oldpage;
+
+ oldpage = phy_select_page(phydev, 7);
+ if (oldpage < 0)
+ return oldpage;
+
+ ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, page);
+ if (ret)
+ return phy_restore_page(phydev, oldpage, ret);
+
+ return oldpage;
+}
+
+static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
+ u32 regnum, u16 mask, u16 set)
+{
+ int ret = 0;
+ int oldpage;
+
+ oldpage = rtl8211e_select_ext_page(phydev, page);
+ if (oldpage < 0)
+ return oldpage;
+
+ ret = __phy_modify(phydev, regnum, mask, set);
+
+ return phy_restore_page(phydev, oldpage, ret);
+}
+
static int rtl8201_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -184,7 +214,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
static int rtl8211e_config_init(struct phy_device *phydev)
{
- int ret = 0, oldpage;
+ int ret;
u16 val;
/* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */
@@ -213,19 +243,9 @@ static int rtl8211e_config_init(struct phy_device *phydev)
* 2 = RX Delay, 1 = TX Delay, 0 = SELRGV (see original PHY datasheet
* for details).
*/
- oldpage = phy_select_page(phydev, 0x7);
- if (oldpage < 0)
- goto err_restore_page;
-
- ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
- if (ret)
- goto err_restore_page;
-
- ret = __phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
- val);
-
-err_restore_page:
- return phy_restore_page(phydev, oldpage, ret);
+ return rtl8211e_modify_ext_paged(phydev, 0xa4, 0x1c,
+ RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
+ val);
}
static int rtl8211b_suspend(struct phy_device *phydev)
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* [PATCH v4 4/4] net: phy: realtek: configure RTL8211E LEDs
From: Matthias Kaehlcke @ 2019-08-01 19:07 UTC (permalink / raw)
To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
Florian Fainelli, Heiner Kallweit
Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
Matthias Kaehlcke
In-Reply-To: <20190801190759.28201-1-mka@chromium.org>
Configure the RTL8211E LEDs behavior when the device tree property
'realtek,led-modes' is specified.
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v4:
- use the generic PHY LED binding
- keep default/current configuration if none is specified
- added rtl8211e_disable_eee_led_mode()
- was previously in separate patch, however since we always want to
disable EEE LED mode when a LED configuration is specified it makes
sense to just add the function here.
- don't call phy_restore_page() in rtl8211e_config_leds() if
selection of the extended page failed.
- use phydev_warn() instead of phydev_err() if LED configuration
fails since we don't bail out
- use hex number to specify page for consistency
- add hex number to comment about ext page 44 to facilitate searching
---
drivers/net/phy/realtek.c | 121 +++++++++++++++++++++++++++++++++++++-
1 file changed, 120 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index e09d3b0da2c7..46e3d77d41b6 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -9,8 +9,11 @@
* Copyright (c) 2004 Freescale Semiconductor, Inc.
*/
#include <linux/bitops.h>
-#include <linux/phy.h>
+#include <linux/bits.h>
#include <linux/module.h>
+#include <linux/phy.h>
+
+#define RTL821x_NUM_LEDS 3
#define RTL821x_PHYSR 0x11
#define RTL821x_PHYSR_DUPLEX BIT(13)
@@ -26,6 +29,19 @@
#define RTL821x_EXT_PAGE_SELECT 0x1e
#define RTL821x_PAGE_SELECT 0x1f
+/* RTL8211E page 5 */
+#define RTL8211E_EEE_LED_MODE1 0x05
+#define RTL8211E_EEE_LED_MODE2 0x06
+
+/* RTL8211E extension page 44 (0x2c) */
+#define RTL8211E_LACR 0x1a
+#define RLT8211E_LACR_LEDACTCTRL_SHIFT 4
+#define RLT8211E_LACR_LEDACTCTRL_MASK GENMASK(6, 4)
+#define RTL8211E_LCR 0x1c
+#define RTL8211E_LCR_LEDCTRL_MASK (GENMASK(2, 0) | \
+ GENMASK(6, 4) | \
+ GENMASK(10, 8))
+
#define RTL8211F_INSR 0x1d
#define RTL8211F_TX_DELAY BIT(8)
@@ -83,6 +99,105 @@ static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
return phy_restore_page(phydev, oldpage, ret);
}
+static void rtl8211e_disable_eee_led_mode(struct phy_device *phydev)
+{
+ int oldpage;
+ int err = 0;
+
+ oldpage = phy_select_page(phydev, 5);
+ if (oldpage < 0)
+ goto out;
+
+ /* write magic values to disable EEE LED mode */
+ err = __phy_write(phydev, RTL8211E_EEE_LED_MODE1, 0x8b82);
+ if (err)
+ goto out;
+
+ err = __phy_write(phydev, RTL8211E_EEE_LED_MODE2, 0x052b);
+
+out:
+ phy_restore_page(phydev, oldpage, err);
+}
+
+static int rtl8211e_config_leds(struct phy_device *phydev)
+{
+ int i, oldpage, ret;
+ u16 lacr_bits = 0, lcr_bits = 0;
+ u16 lacr_mask = RLT8211E_LACR_LEDACTCTRL_MASK;
+ u16 lcr_mask = RTL8211E_LCR_LEDCTRL_MASK;
+ bool eed_led_mode_disabled = false;
+
+ for (i = 0; i < RTL821x_NUM_LEDS; i++) {
+ struct phy_led_config cfg;
+
+ ret = of_get_phy_led_cfg(phydev, i, &cfg);
+ if (ret) {
+ lacr_mask &= ~BIT(4 + i);
+ lcr_mask &= ~GENMASK((i * 4) + 2, i * 4);
+ continue;
+ }
+
+ if (!eed_led_mode_disabled) {
+ rtl8211e_disable_eee_led_mode(phydev);
+ eed_led_mode_disabled = true;
+ }
+
+ switch (cfg.trigger) {
+ case PHY_LED_LINK_10M:
+ lcr_bits |= 1 << (i * 4);
+ break;
+
+ case PHY_LED_LINK_100M:
+ lcr_bits |= 2 << (i * 4);
+ break;
+
+ case PHY_LED_LINK_1G:
+ lcr_bits |= 4 << (i * 4);
+ break;
+
+ case PHY_LED_ACTIVITY:
+ lacr_bits |= BIT(RLT8211E_LACR_LEDACTCTRL_SHIFT + i);
+ break;
+
+ default:
+ phydev_warn(phydev,
+ "unknown trigger for LED%d: %d\n",
+ i, cfg.trigger);
+ }
+ }
+
+ oldpage = rtl8211e_select_ext_page(phydev, 0x2c);
+ if (oldpage < 0) {
+ phydev_err(phydev, "failed to select extended page: %d\n", oldpage);
+ return oldpage;
+ }
+
+ if (lacr_mask == 0)
+ goto skip_lacr;
+
+ ret = __phy_modify(phydev, RTL8211E_LACR,
+ lacr_mask, lacr_bits);
+ if (ret) {
+ phydev_err(phydev, "failed to write LACR reg: %d\n",
+ ret);
+ goto err;
+ }
+
+skip_lacr:
+ if (lcr_mask == 0)
+ goto skip_lcr;
+
+ ret = __phy_modify(phydev, RTL8211E_LCR,
+ lcr_mask, lcr_bits);
+ if (ret)
+ phydev_err(phydev, "failed to write LCR reg: %d\n",
+ ret);
+
+skip_lcr:
+err:
+ return phy_restore_page(phydev, oldpage, ret);
+}
+
static int rtl8201_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -217,6 +332,10 @@ static int rtl8211e_config_init(struct phy_device *phydev)
int ret;
u16 val;
+ ret = rtl8211e_config_leds(phydev);
+ if (ret)
+ phydev_warn(phydev, "LED configuration failed: %d\n", ret);
+
/* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */
switch (phydev->interface) {
case PHY_INTERFACE_MODE_RGMII:
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* Re: [PATCH 0/6] hwspinlock: allow sharing of hwspinlocks
From: Bjorn Andersson @ 2019-08-01 19:14 UTC (permalink / raw)
To: Fabien Dessenne
Cc: Ohad Ben-Cohen, Rob Herring, Mark Rutland, Maxime Coquelin,
Alexandre Torgue, Jonathan Corbet, linux-remoteproc, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, linux-doc,
Benjamin Gaignard
In-Reply-To: <1552492237-28810-1-git-send-email-fabien.dessenne@st.com>
On Wed 13 Mar 08:50 PDT 2019, Fabien Dessenne wrote:
> The current implementation does not allow two different devices to use
> a common hwspinlock. This patch set proposes to have, as an option, some
> hwspinlocks shared between several users.
>
> Below is an example that explain the need for this:
> exti: interrupt-controller@5000d000 {
> compatible = "st,stm32mp1-exti", "syscon";
> interrupt-controller;
> #interrupt-cells = <2>;
> reg = <0x5000d000 0x400>;
> hwlocks = <&hsem 1>;
> };
> The two drivers (stm32mp1-exti and syscon) refer to the same hwlock.
> With the current hwspinlock implementation, only the first driver succeeds
> in requesting (hwspin_lock_request_specific) the hwlock. The second request
> fails.
>
>
> The proposed approach does not modify the API, but extends the DT 'hwlocks'
> property with a second optional parameter (the first one identifies an
> hwlock) that specifies whether an hwlock is requested for exclusive usage
> (current behavior) or can be shared between several users.
> Examples:
> hwlocks = <&hsem 8>; Ref to hwlock #8 for exclusive usage
> hwlocks = <&hsem 8 0>; Ref to hwlock #8 for exclusive (0) usage
> hwlocks = <&hsem 8 1>; Ref to hwlock #8 for shared (1) usage
>
> As a constraint, the #hwlock-cells value must be 1 or 2.
> In the current implementation, this can have theorically any value but:
> - all of the exisiting drivers use the same value : 1.
> - the framework supports only one value : 1 (see implementation of
> of_hwspin_lock_simple_xlate())
> Hence, it shall not be a problem to restrict this value to 1 or 2 since
> it won't break any driver.
>
Hi Fabien,
Your series looks good, but it makes me wonder why the hardware locks
should be an exclusive resource.
How about just making all (specific) locks shared?
Regards,
Bjorn
> Fabien Dessenne (6):
> dt-bindings: hwlock: add support of shared locks
> hwspinlock: allow sharing of hwspinlocks
> dt-bindings: hwlock: update STM32 #hwlock-cells value
> ARM: dts: stm32: Add hwspinlock node for stm32mp157 SoC
> ARM: dts: stm32: Add hwlock for irqchip on stm32mp157
> ARM: dts: stm32: hwlocks for GPIO for stm32mp157
>
> .../devicetree/bindings/hwlock/hwlock.txt | 27 +++++--
> .../bindings/hwlock/st,stm32-hwspinlock.txt | 6 +-
> Documentation/hwspinlock.txt | 10 ++-
> arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 2 +
> arch/arm/boot/dts/stm32mp157c.dtsi | 10 +++
> drivers/hwspinlock/hwspinlock_core.c | 82 +++++++++++++++++-----
> drivers/hwspinlock/hwspinlock_internal.h | 2 +
> 7 files changed, 108 insertions(+), 31 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [patch v4 3/5] DT nodes for AST2500 DMA UART driver
From: Rob Herring @ 2019-08-01 19:28 UTC (permalink / raw)
To: sudheer.v
Cc: Greg Kroah-Hartman, Jiri Slaby, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Mark Rutland,
ShivahShankar Shakarnarayan rao, Sudheer V, sudheer veliseti,
linux-kernel@vger.kernel.org, open list:SERIAL DRIVERS,
devicetree, linux-aspeed
In-Reply-To: <1564147640-30753-4-git-send-email-open.sudheer@gmail.com>
On Fri, Jul 26, 2019 at 7:25 AM sudheer.v <open.sudheer@gmail.com> wrote:
>
> From: sudheer veliseti <sudheer.open@gmail.com>
>
> DT node for DMA controller(ast_uart_sdma) doesn't bind to any DMA controller driver.
> This is because Software for DMA controller is not based on DMA framework,but is dedicated
> and serves only UARTs in AST2500. ast_uart_sdma node is searched by compatible string in the
> driver software.basic use of this node is to provide register base address of DMA controller and DMA irq number(<50>).
> IRQ of DMA controller is of crucial importance, which does RX and TX of UART data.
>
> uart nodes dma_uart1,2...etc binds to the platform driver.
> irq numbers <9>,<32>,<33>,<34> in dma_uart nodes install ISRs which are of not much interest in uart data TX/RX .
>
>
> Signed-off-by: sudheer veliseti <sudheer.open@gmail.com>
> ---
>
> changes from v3->v4:
> -
> changes from v2->v3:
> - change logs added
>
> arch/arm/boot/dts/aspeed-ast2500-evb.dts | 21 +++++++
> arch/arm/boot/dts/aspeed-g5.dtsi | 71 ++++++++++++++++++++++--
> 2 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> index 5dbb33c10c4f..4da09fbe94df 100644
> --- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> +++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
> @@ -64,6 +64,27 @@
> status = "okay";
> };
>
> +&ast_uart_sdma {
> + status = "okay";
> +};
> +
> +&dma_uart1 {
> + status = "okay";
> +};
> +
> +&dma_uart2 {
> + status = "okay";
> +};
> +
> +&dma_uart3 {
> + status = "okay";
> +};
> +
> +&dma_uart4 {
> + status = "okay";
> +};
> +
> +
> &mac0 {
> status = "okay";
>
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> index 674746513031..fb7b3ed463de 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -23,10 +23,10 @@
> i2c11 = &i2c11;
> i2c12 = &i2c12;
> i2c13 = &i2c13;
> - serial0 = &uart1;
> - serial1 = &uart2;
> - serial2 = &uart3;
> - serial3 = &uart4;
> + serial0 = &dma_uart1;
> + serial1 = &dma_uart2;
> + serial2 = &dma_uart3;
> + serial3 = &dma_uart4;
> serial4 = &uart5;
> serial5 = &vuart;
> peci0 = &peci0;
> @@ -497,6 +497,69 @@
> status = "disabled";
> };
>
> + ast_uart_sdma: uart_sdma@1e79e000 {
> + compatible = "aspeed,ast-uart-sdma";
> + reg = <0x1e79e000 0x400>;
> + interrupts = <50>;
> + status = "disabled";
> + };
> +
> + dma_uart1: dma_uart1@1e783000{
> + compatible = "aspeed,ast-sdma-uart";
> + reg = <0x1e783000 0x1000>;
Now you have 2 nodes at the same address. That's not valid. Please
build your dtbs with 'W=1' which will warn against this. Adding DMA
support should not be a whole new node. Nodes correspond to h/w
blocks, not drivers.
The old node has a reset, you don't need that? Seems strange too that
only 1 uart has a reset.
> + reg-shift = <2>;
> + interrupts = <9>;
> + clocks = <&syscon ASPEED_CLK_GATE_UART1CLK>;
> + dma-channel = <0>;
This is the channel in ast_uart_sdma? Just because you decided not to
do a DMA engine driver, doesn't mean you can't use the DMA binding.
Considering you need to map clients to the provider, use the DMA
binding.
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_txd1_default
> + &pinctrl_rxd1_default>;
> + status = "disabled";
> + };
> +
> + dma_uart2: dma_uart2@1e78d000{
> + compatible = "aspeed,ast-sdma-uart";
> + reg = <0x1e78d000 0x1000>;
> + reg-shift = <2>;
> + interrupts = <32>;
> + clocks = <&syscon ASPEED_CLK_GATE_UART2CLK>;
> + dma-channel = <1>;
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_txd2_default
> + &pinctrl_rxd2_default>;
> + status = "disabled";
> + };
> +
> + dma_uart3: dma_uart3@1e78e000{
> + compatible = "aspeed,ast-sdma-uart";
> + reg = <0x1e78e000 0x1000>;
> + reg-shift = <2>;
> + interrupts = <33>;
> + clocks = <&syscon ASPEED_CLK_GATE_UART3CLK>;
> + dma-channel = <2>;
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_txd3_default
> + &pinctrl_rxd3_default>;
> + status = "disabled";
> + };
> +
> + dma_uart4: dma_uart4@1e78f000{
> + compatible = "aspeed,ast-sdma-uart";
> + reg = <0x1e78f000 0x1000>;
> + reg-shift = <2>;
> + interrupts = <34>;
> + clocks = <&syscon ASPEED_CLK_GATE_UART4CLK>;
> + dma-channel = <3>;
> + no-loopback-test;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_txd4_default
> + &pinctrl_rxd4_default>;
> + status = "disabled";
> + };
> +
> i2c: bus@1e78a000 {
> compatible = "simple-bus";
> #address-cells = <1>;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v9 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Frank Rowand @ 2019-08-01 19:28 UTC (permalink / raw)
To: Greg Kroah-Hartman, Saravana Kannan
Cc: Rob Herring, Mark Rutland, Rafael J. Wysocki, devicetree,
linux-kernel, David Collins, kernel-team
In-Reply-To: <20190801061209.GA3570@kroah.com>
Hi Greg,
On 7/31/19 11:12 PM, Greg Kroah-Hartman wrote:
> On Wed, Jul 31, 2019 at 03:17:13PM -0700, Saravana Kannan wrote:
>> Add device-links to track functional dependencies between devices
>> after they are created (but before they are probed) by looking at
>> their common DT bindings like clocks, interconnects, etc.
>>
>> Having functional dependencies automatically added before the devices
>> are probed, provides the following benefits:
>>
>> - Optimizes device probe order and avoids the useless work of
>> attempting probes of devices that will not probe successfully
>> (because their suppliers aren't present or haven't probed yet).
>>
>> For example, in a commonly available mobile SoC, registering just
>> one consumer device's driver at an initcall level earlier than the
>> supplier device's driver causes 11 failed probe attempts before the
>> consumer device probes successfully. This was with a kernel with all
>> the drivers statically compiled in. This problem gets a lot worse if
>> all the drivers are loaded as modules without direct symbol
>> dependencies.
>>
>> - Supplier devices like clock providers, interconnect providers, etc
>> need to keep the resources they provide active and at a particular
>> state(s) during boot up even if their current set of consumers don't
>> request the resource to be active. This is because the rest of the
>> consumers might not have probed yet and turning off the resource
>> before all the consumers have probed could lead to a hang or
>> undesired user experience.
>>
>> Some frameworks (Eg: regulator) handle this today by turning off
>> "unused" resources at late_initcall_sync and hoping all the devices
>> have probed by then. This is not a valid assumption for systems with
>> loadable modules. Other frameworks (Eg: clock) just don't handle
>> this due to the lack of a clear signal for when they can turn off
>> resources. This leads to downstream hacks to handle cases like this
>> that can easily be solved in the upstream kernel.
>>
>> By linking devices before they are probed, we give suppliers a clear
>> count of the number of dependent consumers. Once all of the
>> consumers are active, the suppliers can turn off the unused
>> resources without making assumptions about the number of consumers.
>>
>> By default we just add device-links to track "driver presence" (probe
>> succeeded) of the supplier device. If any other functionality provided
>> by device-links are needed, it is left to the consumer/supplier
>> devices to change the link when they probe.
>
> All now queued up in my driver-core-testing branch, and if 0-day is
> happy with this, will move it to my "real" driver-core-next branch in a
> day or so to get included in linux-next.
I have been slow in getting my review out.
This patch series is not yet ready for sending to Linus, so if putting
this in linux-next implies that it will be in your next pull request
to Linus, please do not put it in linux-next.
Thanks,
Frank
>
> thanks for sticking with this!
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH v9 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Greg Kroah-Hartman @ 2019-08-01 19:32 UTC (permalink / raw)
To: Frank Rowand
Cc: Saravana Kannan, Rob Herring, Mark Rutland, Rafael J. Wysocki,
devicetree, linux-kernel, David Collins, kernel-team
In-Reply-To: <5a1e785d-075e-19a0-7d3d-949e1b65d726@gmail.com>
On Thu, Aug 01, 2019 at 12:28:13PM -0700, Frank Rowand wrote:
> Hi Greg,
>
> On 7/31/19 11:12 PM, Greg Kroah-Hartman wrote:
> > On Wed, Jul 31, 2019 at 03:17:13PM -0700, Saravana Kannan wrote:
> >> Add device-links to track functional dependencies between devices
> >> after they are created (but before they are probed) by looking at
> >> their common DT bindings like clocks, interconnects, etc.
> >>
> >> Having functional dependencies automatically added before the devices
> >> are probed, provides the following benefits:
> >>
> >> - Optimizes device probe order and avoids the useless work of
> >> attempting probes of devices that will not probe successfully
> >> (because their suppliers aren't present or haven't probed yet).
> >>
> >> For example, in a commonly available mobile SoC, registering just
> >> one consumer device's driver at an initcall level earlier than the
> >> supplier device's driver causes 11 failed probe attempts before the
> >> consumer device probes successfully. This was with a kernel with all
> >> the drivers statically compiled in. This problem gets a lot worse if
> >> all the drivers are loaded as modules without direct symbol
> >> dependencies.
> >>
> >> - Supplier devices like clock providers, interconnect providers, etc
> >> need to keep the resources they provide active and at a particular
> >> state(s) during boot up even if their current set of consumers don't
> >> request the resource to be active. This is because the rest of the
> >> consumers might not have probed yet and turning off the resource
> >> before all the consumers have probed could lead to a hang or
> >> undesired user experience.
> >>
> >> Some frameworks (Eg: regulator) handle this today by turning off
> >> "unused" resources at late_initcall_sync and hoping all the devices
> >> have probed by then. This is not a valid assumption for systems with
> >> loadable modules. Other frameworks (Eg: clock) just don't handle
> >> this due to the lack of a clear signal for when they can turn off
> >> resources. This leads to downstream hacks to handle cases like this
> >> that can easily be solved in the upstream kernel.
> >>
> >> By linking devices before they are probed, we give suppliers a clear
> >> count of the number of dependent consumers. Once all of the
> >> consumers are active, the suppliers can turn off the unused
> >> resources without making assumptions about the number of consumers.
> >>
> >> By default we just add device-links to track "driver presence" (probe
> >> succeeded) of the supplier device. If any other functionality provided
> >> by device-links are needed, it is left to the consumer/supplier
> >> devices to change the link when they probe.
> >
> > All now queued up in my driver-core-testing branch, and if 0-day is
> > happy with this, will move it to my "real" driver-core-next branch in a
> > day or so to get included in linux-next.
>
> I have been slow in getting my review out.
>
> This patch series is not yet ready for sending to Linus, so if putting
> this in linux-next implies that it will be in your next pull request
> to Linus, please do not put it in linux-next.
It means that it will be in my pull request for 5.4-rc1, many many
waeeks away from now.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 19:42 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <c85ba067-af68-0b4a-d347-501ed7ed0ef9@gmail.com>
On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>> This patch implements save and restore context for peripheral fixed
>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>> peripheral clock ops.
>>>>>>
>>>>>> During system suspend, core power goes off and looses the settings
>>>>>> of the Tegra CAR controller registers.
>>>>>>
>>>>>> So during suspend entry clock and reset state of peripherals is saved
>>>>>> and on resume they are restored to have clocks back to same rate and
>>>>>> state as before suspend.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>> +++++++++++++++++++++++++++++++++
>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>> +++++++++++++++++++++++++++
>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>> 5 files changed, 138 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>> clk_hw *hw,
>>>>>> return (unsigned long)rate;
>>>>>> }
>>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>> +
>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>> fixed->regs->enb_reg) &
>>>>>> + mask;
>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>> fixed->regs->rst_reg) &
>>>>>> + mask;
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>> +
>>>>>> + if (fixed->enb_ctx)
>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_set_reg);
>>>>>> + else
>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->enb_clr_reg);
>>>>>> +
>>>>>> + udelay(2);
>>>>>> +
>>>>>> + if (!fixed->rst_ctx) {
>>>>>> + udelay(5); /* reset propogation delay */
>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>> };
>>>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>> #define read_rst(gate) \
>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>> +#define write_rst_set(val, gate) \
>>>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>>>> #define write_rst_clr(val, gate) \
>>>>>> writel_relaxed(val, gate->clk_base + (gate->regs->rst_clr_reg))
>>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>>> clk_hw *hw)
>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>> }
>>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>> +
>>>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>> +
>>>>>> + if (gate->clk_state_ctx)
>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>> + else
>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>> +
>>>>>> + udelay(5);
>>>>>> +
>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>> + if (gate->rst_state_ctx)
>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>> + else
>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>> .enable = clk_periph_enable,
>>>>>> .disable = clk_periph_disable,
>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>> };
>>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>> index 58437da25156..06fb62955768 100644
>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>>>> gate_ops->disable(gate_hw);
>>>>>> }
>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>> +
>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>> + gate_ops->save_context(gate_hw);
>>>>>> +
>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>> +{
>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>> +
>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>> +
>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>> + div_ops->restore_context(div_hw);
>>>>> Could you please point to where the divider's save_context() happens?
>>>>> Because I can't see it.
>>>> Ah, I now see that there is no need to save the dividers context because
>>>> clk itself has enough info that is needed for the context's restoring
>>>> (like I pointed in the review to v6).
>>>>
>>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>>> generic helper to get the index instead of storing it manually.
>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>> get_parent() which is then saved in tegra_clk_periph.
>>>
>>> All existing drivers are using directly get_parent() from clk_mux
>>> which actually gets index from the register read.
>>>
>>> To have this more generic w.r.t save/restore context point of view,
>>> probably instead of implementing new get_parent_index helper, I think
>>> its better to implement save_context and restore_context to
>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>> cache index during set_parent.
>>>
>>> So we just need to invoke mux_ops save_context and restore_context.
>>>
>> I hope its ok to add save/restore context to clk_mux_ops to be more
>> generic w.r.t save/restore context rather than get_parent_index API.
>> Please confirm if you agree.
> Sounds like a good idea. I see that there is a 'restoring' helper for
> the generic clk_gate, seems something similar could be done for the
> clk_mux. And looks like anyway you'll need to associate the parent clock
> with the hw index in order to restore the muxing.
by 'restoring' helper for generic clk_gate, are you referring to
clk_gate_restore_context API?
clk_gate_restore_context is API that's any clk drivers can use for
clk_gate operation restore for custom gate clk_ops.
But clk-periph is directly using generic clk_mux ops from clk_mux so I
think we should add .restore_context to clk_mux_ops and then during
clk-periph restore need to invoke mux_ops->restore_context.
^ permalink raw reply
* Re: [PATCH] scripts/dtc: dtx_diff - add color output support
From: Frank Rowand @ 2019-08-01 19:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CAMuHMdWPvD_pSyJGp=kC0XmAChCK8R2X+exmpHT5eywJ5kQetA@mail.gmail.com>
On 8/1/19 5:13 AM, Geert Uytterhoeven wrote:
> Hi Frank,
>
> On Wed, Jul 31, 2019 at 10:30 PM Frank Rowand <frowand.list@gmail.com> wrote:
>> On 7/31/19 5:37 AM, Geert Uytterhoeven wrote:
>>> Add new -c/--color options, to enhance the diff output with color, and
>>> improve the user's experience.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> scripts/dtc/dtx_diff | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff
>>> index e9ad7834a22d9459..4e2c8617f69a333e 100755
>>> --- a/scripts/dtc/dtx_diff
>>> +++ b/scripts/dtc/dtx_diff
>>> @@ -20,6 +20,8 @@ Usage:
>>>
>>>
>>> --annotate synonym for -T
>>> + --color synonym for -c
>>> + -c enable colored output
>>> -f print full dts in diff (--unified=99999)
>>> -h synonym for --help
>>> -help synonym for --help
>
>> I like the idea, but...
>>
>> I have various linux distro releases across my many systems, but only one is
>> new enough to have the diff command that supports --color.
>
> Seems to have been added in diffutils release 3.4 (2016-08-08).
> I almost can't believe it was that recent, but then I remembered using a
> wrapper before (colordiff; other wrappers may exist).
>
>> Can you enhance this patch to test whether --color is supported? Maybe
>> something like (untested):
>>
>> -c | --color )
>> if `diff --color <(echo a) <(echo a) 2>/dev/null` ; then
>> diff_color="--color=always"
>> fi
>> shift
>> ;;
>>
>> Then add some text to the usage for -c and --color saying that they will
>> be silently ignored if diff does not support --color.
>>
>> I first wrote up a suggested version that printed an error message and
>> exited, but I think silently ignoring is more robust, even though it
>> may be more confusing to someone who is wondering why --color does not
>> work.
>
> Given this is an optional feature, to be enabled explicitly by the user,
> I'm not so fond of going through hoops to auto-detect the availability.
>
> So what about just documenting this in the help text instead?
>
> - -c enable colored output
> + -c enable colored output (requires diff with --color support)
----- thought 1 -----
My first thought was:
If the hoops were complex and ugly, I might agree with you. But since it is
a simple one line "if" (two lines including "fi") I prefer the check.
The help text update looks good to me, along with the check.
----- thought 2 -----
Then I reconsidered, and thought "well, Geert has a good idea". So I
decided to see how useful the diff error message would be. The message is:
$ scripts/dtc/dtx_diff -c a.dts b.dts
diff: unrecognized option '--color=always'
diff: Try 'diff --help' for more information.
$
Possible hints to resolve the above error:
(hints might not fix the problem)
No hints available.
It is interesting that the shell prompt arrives before the full set of
messages from the script, but that is not my issue. My issue is that
when the diff fails, the script tries to find suggestions to solve
the problem. (The suggestions exist to catch some likely problems
with the shell variable "ARCH".)
Unfortunately in the case of the "--color" problem, the useful warning
from diff becomes less visible because of the early prompt and the
not so helpful messages about hints.
If the hints related messages were not present, then I was ready to
accept that the diff warning was sufficient. But since the hints
messages are present, and hiding them would be more complex than
the original check that I suggested for whether diff supports
the --color option, I am back to my first thought: I prefer the
check whether diff supports "--color" is done when dtx_diff detects
the "--color" option.
-Frank
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH v9 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Frank Rowand @ 2019-08-01 19:59 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Saravana Kannan, Rob Herring, Mark Rutland, Rafael J. Wysocki,
devicetree, linux-kernel, David Collins, kernel-team
In-Reply-To: <20190801193248.GA24916@kroah.com>
On 8/1/19 12:32 PM, Greg Kroah-Hartman wrote:
> On Thu, Aug 01, 2019 at 12:28:13PM -0700, Frank Rowand wrote:
>> Hi Greg,
>>
>> On 7/31/19 11:12 PM, Greg Kroah-Hartman wrote:
>>> On Wed, Jul 31, 2019 at 03:17:13PM -0700, Saravana Kannan wrote:
>>>> Add device-links to track functional dependencies between devices
>>>> after they are created (but before they are probed) by looking at
>>>> their common DT bindings like clocks, interconnects, etc.
>>>>
>>>> Having functional dependencies automatically added before the devices
>>>> are probed, provides the following benefits:
>>>>
>>>> - Optimizes device probe order and avoids the useless work of
>>>> attempting probes of devices that will not probe successfully
>>>> (because their suppliers aren't present or haven't probed yet).
>>>>
>>>> For example, in a commonly available mobile SoC, registering just
>>>> one consumer device's driver at an initcall level earlier than the
>>>> supplier device's driver causes 11 failed probe attempts before the
>>>> consumer device probes successfully. This was with a kernel with all
>>>> the drivers statically compiled in. This problem gets a lot worse if
>>>> all the drivers are loaded as modules without direct symbol
>>>> dependencies.
>>>>
>>>> - Supplier devices like clock providers, interconnect providers, etc
>>>> need to keep the resources they provide active and at a particular
>>>> state(s) during boot up even if their current set of consumers don't
>>>> request the resource to be active. This is because the rest of the
>>>> consumers might not have probed yet and turning off the resource
>>>> before all the consumers have probed could lead to a hang or
>>>> undesired user experience.
>>>>
>>>> Some frameworks (Eg: regulator) handle this today by turning off
>>>> "unused" resources at late_initcall_sync and hoping all the devices
>>>> have probed by then. This is not a valid assumption for systems with
>>>> loadable modules. Other frameworks (Eg: clock) just don't handle
>>>> this due to the lack of a clear signal for when they can turn off
>>>> resources. This leads to downstream hacks to handle cases like this
>>>> that can easily be solved in the upstream kernel.
>>>>
>>>> By linking devices before they are probed, we give suppliers a clear
>>>> count of the number of dependent consumers. Once all of the
>>>> consumers are active, the suppliers can turn off the unused
>>>> resources without making assumptions about the number of consumers.
>>>>
>>>> By default we just add device-links to track "driver presence" (probe
>>>> succeeded) of the supplier device. If any other functionality provided
>>>> by device-links are needed, it is left to the consumer/supplier
>>>> devices to change the link when they probe.
>>>
>>> All now queued up in my driver-core-testing branch, and if 0-day is
>>> happy with this, will move it to my "real" driver-core-next branch in a
>>> day or so to get included in linux-next.
>>
>> I have been slow in getting my review out.
>>
>> This patch series is not yet ready for sending to Linus, so if putting
>> this in linux-next implies that it will be in your next pull request
>> to Linus, please do not put it in linux-next.
>
> It means that it will be in my pull request for 5.4-rc1, many many
> waeeks away from now.
If you are willing to revert the series before the pull request _if_ I
have significant review issues in the next couple of days, then I am happy
to see the patches get exposure in linux-next.
-Frank
>
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 20:17 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <a81b85a2-5634-cfa2-77c5-94c23c4847bd@nvidia.com>
01.08.2019 22:42, Sowjanya Komatineni пишет:
>
> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>> This patch implements save and restore context for peripheral fixed
>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>>> peripheral clock ops.
>>>>>>>
>>>>>>> During system suspend, core power goes off and looses the settings
>>>>>>> of the Tegra CAR controller registers.
>>>>>>>
>>>>>>> So during suspend entry clock and reset state of peripherals is
>>>>>>> saved
>>>>>>> and on resume they are restored to have clocks back to same rate and
>>>>>>> state as before suspend.
>>>>>>>
>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>> ---
>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>> +++++++++++++++++++++++++++
>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>> clk_hw *hw,
>>>>>>> return (unsigned long)rate;
>>>>>>> }
>>>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw
>>>>>>> *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>> +
>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>> fixed->regs->enb_reg) &
>>>>>>> + mask;
>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>> fixed->regs->rst_reg) &
>>>>>>> + mask;
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw
>>>>>>> *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>> +
>>>>>>> + if (fixed->enb_ctx)
>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>> fixed->regs->enb_set_reg);
>>>>>>> + else
>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>> +
>>>>>>> + udelay(2);
>>>>>>> +
>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>>> };
>>>>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>> #define read_rst(gate) \
>>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>>>> clk_hw *hw)
>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>> }
>>>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>> +
>>>>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>> +
>>>>>>> + if (gate->clk_state_ctx)
>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>> + else
>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>> +
>>>>>>> + udelay(5);
>>>>>>> +
>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>> + if (gate->rst_state_ctx)
>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>> + else
>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>> + }
>>>>>>> +}
>>>>>>> +
>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>> .enable = clk_periph_enable,
>>>>>>> .disable = clk_periph_disable,
>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>> };
>>>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>>>>> gate_ops->disable(gate_hw);
>>>>>>> }
>>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>> +
>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>> +
>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>>> +{
>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>> +
>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>> +
>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>> + div_ops->restore_context(div_hw);
>>>>>> Could you please point to where the divider's save_context() happens?
>>>>>> Because I can't see it.
>>>>> Ah, I now see that there is no need to save the dividers context
>>>>> because
>>>>> clk itself has enough info that is needed for the context's restoring
>>>>> (like I pointed in the review to v6).
>>>>>
>>>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>>>> generic helper to get the index instead of storing it manually.
>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>
>>>> All existing drivers are using directly get_parent() from clk_mux
>>>> which actually gets index from the register read.
>>>>
>>>> To have this more generic w.r.t save/restore context point of view,
>>>> probably instead of implementing new get_parent_index helper, I think
>>>> its better to implement save_context and restore_context to
>>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>>> cache index during set_parent.
>>>>
>>>> So we just need to invoke mux_ops save_context and restore_context.
>>>>
>>> I hope its ok to add save/restore context to clk_mux_ops to be more
>>> generic w.r.t save/restore context rather than get_parent_index API.
>>> Please confirm if you agree.
>> Sounds like a good idea. I see that there is a 'restoring' helper for
>> the generic clk_gate, seems something similar could be done for the
>> clk_mux. And looks like anyway you'll need to associate the parent clock
>> with the hw index in order to restore the muxing.
>
> by 'restoring' helper for generic clk_gate, are you referring to
> clk_gate_restore_context API?
Yes.
> clk_gate_restore_context is API that's any clk drivers can use for
> clk_gate operation restore for custom gate clk_ops.
>
> But clk-periph is directly using generic clk_mux ops from clk_mux so I
> think we should add .restore_context to clk_mux_ops and then during
> clk-periph restore need to invoke mux_ops->restore_context.
I'm not sure whether it will be good for every driver that uses generic
clk_mux ops. Should be more flexible to have a generic helper function
that any driver could use in order to restore the clock's parent.
The clk-periph restoring also includes case of combining divider and
parent restoring, so generic helper could be useful in that case as well.
It also looks like you could actually use the clk_gate_restore_context()
instead of manually saving the clock's enable-state, couldn't you?
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 20:31 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <ef9e865f-359b-0873-a414-3d548bd4e590@gmail.com>
On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>> This patch implements save and restore context for peripheral fixed
>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>>>> peripheral clock ops.
>>>>>>>>
>>>>>>>> During system suspend, core power goes off and looses the settings
>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>
>>>>>>>> So during suspend entry clock and reset state of peripherals is
>>>>>>>> saved
>>>>>>>> and on resume they are restored to have clocks back to same rate and
>>>>>>>> state as before suspend.
>>>>>>>>
>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>> ---
>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>> clk_hw *hw,
>>>>>>>> return (unsigned long)rate;
>>>>>>>> }
>>>>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw
>>>>>>>> *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>> +
>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>> + mask;
>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>> + mask;
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw
>>>>>>>> *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>> +
>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>> + else
>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>> +
>>>>>>>> + udelay(2);
>>>>>>>> +
>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>>>>> + }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>>>> };
>>>>>>>> struct clk *tegra_clk_register_periph_fixed(const char *name,
>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>> #define read_rst(gate) \
>>>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>> + writel_relaxed(val, gate->clk_base + (gate->regs->rst_set_reg))
>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>>>>> clk_hw *hw)
>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>> }
>>>>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>> +
>>>>>>>> + gate->clk_state_ctx = read_enb(gate) & periph_clk_to_bit(gate);
>>>>>>>> + gate->rst_state_ctx = read_rst(gate) & periph_clk_to_bit(gate);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>> +
>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>> + else
>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>> +
>>>>>>>> + udelay(5);
>>>>>>>> +
>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>> + else
>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>> + }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>> .enable = clk_periph_enable,
>>>>>>>> .disable = clk_periph_disable,
>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>> };
>>>>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw *hw)
>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>> }
>>>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>> +
>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>> +
>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>>>> +{
>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>> +
>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>> +
>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>> Could you please point to where the divider's save_context() happens?
>>>>>>> Because I can't see it.
>>>>>> Ah, I now see that there is no need to save the dividers context
>>>>>> because
>>>>>> clk itself has enough info that is needed for the context's restoring
>>>>>> (like I pointed in the review to v6).
>>>>>>
>>>>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>>>>> generic helper to get the index instead of storing it manually.
>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>
>>>>> All existing drivers are using directly get_parent() from clk_mux
>>>>> which actually gets index from the register read.
>>>>>
>>>>> To have this more generic w.r.t save/restore context point of view,
>>>>> probably instead of implementing new get_parent_index helper, I think
>>>>> its better to implement save_context and restore_context to
>>>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>>>> cache index during set_parent.
>>>>>
>>>>> So we just need to invoke mux_ops save_context and restore_context.
>>>>>
>>>> I hope its ok to add save/restore context to clk_mux_ops to be more
>>>> generic w.r.t save/restore context rather than get_parent_index API.
>>>> Please confirm if you agree.
>>> Sounds like a good idea. I see that there is a 'restoring' helper for
>>> the generic clk_gate, seems something similar could be done for the
>>> clk_mux. And looks like anyway you'll need to associate the parent clock
>>> with the hw index in order to restore the muxing.
>> by 'restoring' helper for generic clk_gate, are you referring to
>> clk_gate_restore_context API?
> Yes.
>
>> clk_gate_restore_context is API that's any clk drivers can use for
>> clk_gate operation restore for custom gate clk_ops.
>>
>> But clk-periph is directly using generic clk_mux ops from clk_mux so I
>> think we should add .restore_context to clk_mux_ops and then during
>> clk-periph restore need to invoke mux_ops->restore_context.
> I'm not sure whether it will be good for every driver that uses generic
> clk_mux ops. Should be more flexible to have a generic helper function
> that any driver could use in order to restore the clock's parent.
>
> The clk-periph restoring also includes case of combining divider and
> parent restoring, so generic helper could be useful in that case as well.
>
> It also looks like you could actually use the clk_gate_restore_context()
> instead of manually saving the clock's enable-state, couldn't you?
ok for clk_mux, can add generic clk_mux_restore_context API rather than
using restore_context in clk_ops and will invoke that during clk_periph
restore.
Reg clk_gate, looks like we cant use generic clk_gate_restore_context
for clk-periph as it calls enable/disable callbacks and
clk_periph_enable/disable in clk-periph-gate also updated refcnt and
depending on that actual enable/disable is set.
During suspend, peripherals that are already enabled have their refcnt >
1, so they dont go thru enable/disable on restore if we use same
enable/disable callback.
Also to align exact reset state along with CLK (like for case where CLK
is enabled but peripheral might be in reset state), implemented
save/restore in tegra specific tegra_clk_periph_gate_ops
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 20:54 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <50bad1d3-df41-d1e5-a7c7-4be9c661ed14@nvidia.com>
01.08.2019 23:31, Sowjanya Komatineni пишет:
>
> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>> This patch implements save and restore context for peripheral
>>>>>>>>> fixed
>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>>>>> peripheral clock ops.
>>>>>>>>>
>>>>>>>>> During system suspend, core power goes off and looses the settings
>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>
>>>>>>>>> So during suspend entry clock and reset state of peripherals is
>>>>>>>>> saved
>>>>>>>>> and on resume they are restored to have clocks back to same
>>>>>>>>> rate and
>>>>>>>>> state as before suspend.
>>>>>>>>>
>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>> ---
>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>> clk_hw *hw,
>>>>>>>>> return (unsigned long)rate;
>>>>>>>>> }
>>>>>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw
>>>>>>>>> *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>> +
>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>> + mask;
>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>> + mask;
>>>>>>>>> +
>>>>>>>>> + return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw
>>>>>>>>> *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>> +
>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>> + else
>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>> +
>>>>>>>>> + udelay(2);
>>>>>>>>> +
>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>>>>>> + }
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>>>>> };
>>>>>>>>> struct clk *tegra_clk_register_periph_fixed(const char
>>>>>>>>> *name,
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>> #define read_rst(gate) \
>>>>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>>>>>> clk_hw *hw)
>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>> }
>>>>>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>>> +
>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>> +
>>>>>>>>> + return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>>> +
>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>> + else
>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>> +
>>>>>>>>> + udelay(5);
>>>>>>>>> +
>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>> + else
>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>> + }
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>>> };
>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw
>>>>>>>>> *hw)
>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>> }
>>>>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>> +
>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>> +
>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>> +
>>>>>>>>> + return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>>>>> +{
>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>> +
>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>> +
>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>> Could you please point to where the divider's save_context()
>>>>>>>> happens?
>>>>>>>> Because I can't see it.
>>>>>>> Ah, I now see that there is no need to save the dividers context
>>>>>>> because
>>>>>>> clk itself has enough info that is needed for the context's
>>>>>>> restoring
>>>>>>> (like I pointed in the review to v6).
>>>>>>>
>>>>>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>>>>>> generic helper to get the index instead of storing it manually.
>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>
>>>>>> All existing drivers are using directly get_parent() from clk_mux
>>>>>> which actually gets index from the register read.
>>>>>>
>>>>>> To have this more generic w.r.t save/restore context point of view,
>>>>>> probably instead of implementing new get_parent_index helper, I think
>>>>>> its better to implement save_context and restore_context to
>>>>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>>>>> cache index during set_parent.
>>>>>>
>>>>>> So we just need to invoke mux_ops save_context and restore_context.
>>>>>>
>>>>> I hope its ok to add save/restore context to clk_mux_ops to be more
>>>>> generic w.r.t save/restore context rather than get_parent_index API.
>>>>> Please confirm if you agree.
>>>> Sounds like a good idea. I see that there is a 'restoring' helper for
>>>> the generic clk_gate, seems something similar could be done for the
>>>> clk_mux. And looks like anyway you'll need to associate the parent
>>>> clock
>>>> with the hw index in order to restore the muxing.
>>> by 'restoring' helper for generic clk_gate, are you referring to
>>> clk_gate_restore_context API?
>> Yes.
>>
>>> clk_gate_restore_context is API that's any clk drivers can use for
>>> clk_gate operation restore for custom gate clk_ops.
>>>
>>> But clk-periph is directly using generic clk_mux ops from clk_mux so I
>>> think we should add .restore_context to clk_mux_ops and then during
>>> clk-periph restore need to invoke mux_ops->restore_context.
>> I'm not sure whether it will be good for every driver that uses generic
>> clk_mux ops. Should be more flexible to have a generic helper function
>> that any driver could use in order to restore the clock's parent.
>>
>> The clk-periph restoring also includes case of combining divider and
>> parent restoring, so generic helper could be useful in that case as well.
>>
>> It also looks like you could actually use the clk_gate_restore_context()
>> instead of manually saving the clock's enable-state, couldn't you?
>
> ok for clk_mux, can add generic clk_mux_restore_context API rather than
> using restore_context in clk_ops and will invoke that during clk_periph
> restore.
>
>
> Reg clk_gate, looks like we cant use generic clk_gate_restore_context
> for clk-periph as it calls enable/disable callbacks and
> clk_periph_enable/disable in clk-periph-gate also updated refcnt and
> depending on that actual enable/disable is set.
>
> During suspend, peripherals that are already enabled have their refcnt >
> 1, so they dont go thru enable/disable on restore if we use same
> enable/disable callback.
Looks like you could just decrement the gate's enable_refcnt on
save_context, wouldn't that work?
>
> Also to align exact reset state along with CLK (like for case where CLK
> is enabled but peripheral might be in reset state), implemented
> save/restore in tegra specific tegra_clk_periph_gate_ops
I'm wondering whether instead of saving/restoring reset-state of every
clock, you could simply save/restore the whole RST_DEV_x_SET register.
Couldn't you?
^ permalink raw reply
* Re: [PATCH v4 4/4] net: phy: realtek: configure RTL8211E LEDs
From: David Miller @ 2019-08-01 21:04 UTC (permalink / raw)
To: mka
Cc: robh+dt, mark.rutland, andrew, f.fainelli, hkallweit1, netdev,
devicetree, linux-kernel, dianders
In-Reply-To: <20190801190759.28201-5-mka@chromium.org>
From: Matthias Kaehlcke <mka@chromium.org>
Date: Thu, 1 Aug 2019 12:07:59 -0700
> +static int rtl8211e_config_leds(struct phy_device *phydev)
> +{
> + int i, oldpage, ret;
> + u16 lacr_bits = 0, lcr_bits = 0;
> + u16 lacr_mask = RLT8211E_LACR_LEDACTCTRL_MASK;
> + u16 lcr_mask = RTL8211E_LCR_LEDCTRL_MASK;
> + bool eed_led_mode_disabled = false;
Please use reverse christmas tree ordering here.
^ permalink raw reply
* Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Stephen Boyd @ 2019-08-01 21:14 UTC (permalink / raw)
To: Brendan Higgins
Cc: devicetree, Peter Zijlstra, Amir Goldstein, dri-devel,
Sasha Levin, Masahiro Yamada, Michael Ellerman,
open list:KERNEL SELFTEST FRAMEWORK, shuah, Rob Herring,
linux-nvdimm, Timothy Bird, Frank Rowand, open list:DOCUMENTATION,
Knut Omang, Kieran Bingham, wfg-VuQAYsv1563Yd54FQh9/CA,
Joel Stanley, David Rientjes, Kevin Hilman, Dan Carpenter,
Petr Mladek
In-Reply-To: <CAFd5g473iFfvBnJs2pcwuJYgY+DpgD6RLzyDFL1otUuScgKUag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Quoting Brendan Higgins (2019-08-01 11:59:57)
> On Thu, Aug 1, 2019 at 11:55 AM Brendan Higgins
> <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > On Fri, Jul 26, 2019 at 1:31 AM Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org> wrote:
> >
> > > To be honest I do not fully understand KUnit design. I am not
> > > completely sure how the tested code is isolated from the running
> > > system. Namely, I do not know if the tested code shares
> > > the same locks with the system running the test.
> >
> > No worries, I don't expect printk to be the hang up in those cases. It
> > sounds like KUnit has a long way to evolve before printk is going to
> > be a limitation.
>
> So Stephen, what do you think?
>
> Do you want me to go forward with the new kunit_assert API wrapping
> the string_stream as I have it now? Would you prefer to punt this to a
> later patch? Or would you prefer something else?
>
I like the struct based approach. If anything, it can be adjusted to
make the code throw some records into a spinlock later on and delay the
formatting of the assertion if need be. Can you resend with that
approach? I don't think I'll have any more comments after that.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 21:30 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <62a5c6ed-21b1-8403-6fac-9c5d99b5a255@gmail.com>
On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>> This patch implements save and restore context for peripheral
>>>>>>>>>> fixed
>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>
>>>>>>>>>> During system suspend, core power goes off and looses the settings
>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>
>>>>>>>>>> So during suspend entry clock and reset state of peripherals is
>>>>>>>>>> saved
>>>>>>>>>> and on resume they are restored to have clocks back to same
>>>>>>>>>> rate and
>>>>>>>>>> state as before suspend.
>>>>>>>>>>
>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>> clk_hw *hw,
>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>> }
>>>>>>>>>> +static int tegra_clk_periph_fixed_save_context(struct clk_hw
>>>>>>>>>> *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>> +
>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>> + mask;
>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>> + mask;
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct clk_hw
>>>>>>>>>> *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>> +
>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>> + else
>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>> +
>>>>>>>>>> + udelay(2);
>>>>>>>>>> +
>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>> + writel_relaxed(mask, fixed->base + fixed->regs->rst_reg);
>>>>>>>>>> + }
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>>>>>> };
>>>>>>>>>> struct clk *tegra_clk_register_periph_fixed(const char
>>>>>>>>>> *name,
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>> @@ -110,10 +112,42 @@ static void clk_periph_disable(struct
>>>>>>>>>> clk_hw *hw)
>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>> }
>>>>>>>>>> +static int clk_periph_gate_save_context(struct clk_hw *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>>>> +
>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw);
>>>>>>>>>> +
>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>> + else
>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>> +
>>>>>>>>>> + udelay(5);
>>>>>>>>>> +
>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>> + else
>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>> + }
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>>>> };
>>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const char *name,
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw
>>>>>>>>>> *hw)
>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>> }
>>>>>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>> +
>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>> +
>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>>>>>> +{
>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>> +
>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>> +
>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>> Could you please point to where the divider's save_context()
>>>>>>>>> happens?
>>>>>>>>> Because I can't see it.
>>>>>>>> Ah, I now see that there is no need to save the dividers context
>>>>>>>> because
>>>>>>>> clk itself has enough info that is needed for the context's
>>>>>>>> restoring
>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>
>>>>>>>> Looks like you could also implement a new clk_hw_get_parent_index()
>>>>>>>> generic helper to get the index instead of storing it manually.
>>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>
>>>>>>> All existing drivers are using directly get_parent() from clk_mux
>>>>>>> which actually gets index from the register read.
>>>>>>>
>>>>>>> To have this more generic w.r.t save/restore context point of view,
>>>>>>> probably instead of implementing new get_parent_index helper, I think
>>>>>>> its better to implement save_context and restore_context to
>>>>>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>>>>>> cache index during set_parent.
>>>>>>>
>>>>>>> So we just need to invoke mux_ops save_context and restore_context.
>>>>>>>
>>>>>> I hope its ok to add save/restore context to clk_mux_ops to be more
>>>>>> generic w.r.t save/restore context rather than get_parent_index API.
>>>>>> Please confirm if you agree.
>>>>> Sounds like a good idea. I see that there is a 'restoring' helper for
>>>>> the generic clk_gate, seems something similar could be done for the
>>>>> clk_mux. And looks like anyway you'll need to associate the parent
>>>>> clock
>>>>> with the hw index in order to restore the muxing.
>>>> by 'restoring' helper for generic clk_gate, are you referring to
>>>> clk_gate_restore_context API?
>>> Yes.
>>>
>>>> clk_gate_restore_context is API that's any clk drivers can use for
>>>> clk_gate operation restore for custom gate clk_ops.
>>>>
>>>> But clk-periph is directly using generic clk_mux ops from clk_mux so I
>>>> think we should add .restore_context to clk_mux_ops and then during
>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>> I'm not sure whether it will be good for every driver that uses generic
>>> clk_mux ops. Should be more flexible to have a generic helper function
>>> that any driver could use in order to restore the clock's parent.
>>>
>>> The clk-periph restoring also includes case of combining divider and
>>> parent restoring, so generic helper could be useful in that case as well.
>>>
>>> It also looks like you could actually use the clk_gate_restore_context()
>>> instead of manually saving the clock's enable-state, couldn't you?
>> ok for clk_mux, can add generic clk_mux_restore_context API rather than
>> using restore_context in clk_ops and will invoke that during clk_periph
>> restore.
>>
>>
>> Reg clk_gate, looks like we cant use generic clk_gate_restore_context
>> for clk-periph as it calls enable/disable callbacks and
>> clk_periph_enable/disable in clk-periph-gate also updated refcnt and
>> depending on that actual enable/disable is set.
>>
>> During suspend, peripherals that are already enabled have their refcnt >
>> 1, so they dont go thru enable/disable on restore if we use same
>> enable/disable callback.
> Looks like you could just decrement the gate's enable_refcnt on
> save_context, wouldn't that work?
>
>> Also to align exact reset state along with CLK (like for case where CLK
>> is enabled but peripheral might be in reset state), implemented
>> save/restore in tegra specific tegra_clk_periph_gate_ops
> I'm wondering whether instead of saving/restoring reset-state of every
> clock, you could simply save/restore the whole RST_DEV_x_SET register.
> Couldn't you?
Thats what I was doing in first version of patch. But later as we moved
to use clk_save_context and clk_restore_context, peripheral clk_hw RST &
CLK enables happen thru its corresponding save/restore after source restore
^ permalink raw reply
* Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Brendan Higgins @ 2019-08-01 21:43 UTC (permalink / raw)
To: Stephen Boyd
Cc: Petr Mladek, Jeff Dike, Kevin Hilman, Logan Gunthorpe,
Michael Ellerman, Daniel Vetter, Amir Goldstein, Frank Rowand,
Steven Rostedt, Kees Cook, David Rientjes, kunit-dev,
Kieran Bingham, Peter Zijlstra, Randy Dunlap, Joel Stanley,
Luis Chamberlain, Rob Herring, shuah, wfg, Greg KH,
Julia Lawall <ju>
In-Reply-To: <20190801211447.6D3D7206A2@mail.kernel.org>
On Thu, Aug 1, 2019 at 2:14 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Brendan Higgins (2019-08-01 11:59:57)
> > On Thu, Aug 1, 2019 at 11:55 AM Brendan Higgins
> > <brendanhiggins@google.com> wrote:
> > >
> > > On Fri, Jul 26, 2019 at 1:31 AM Petr Mladek <pmladek@suse.com> wrote:
> > >
> > > > To be honest I do not fully understand KUnit design. I am not
> > > > completely sure how the tested code is isolated from the running
> > > > system. Namely, I do not know if the tested code shares
> > > > the same locks with the system running the test.
> > >
> > > No worries, I don't expect printk to be the hang up in those cases. It
> > > sounds like KUnit has a long way to evolve before printk is going to
> > > be a limitation.
> >
> > So Stephen, what do you think?
> >
> > Do you want me to go forward with the new kunit_assert API wrapping
> > the string_stream as I have it now? Would you prefer to punt this to a
> > later patch? Or would you prefer something else?
> >
>
> I like the struct based approach. If anything, it can be adjusted to
> make the code throw some records into a spinlock later on and delay the
> formatting of the assertion if need be.
That's a fair point.
> Can you resend with that
> approach? I don't think I'll have any more comments after that.
Cool, will do.
Thanks!
^ permalink raw reply
* [PATCH] ARM: dts: rockchip: Add pin names for rk3288-veyron fievel
From: Matthias Kaehlcke @ 2019-08-01 22:03 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Heiko Stuebner
Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
Douglas Anderson, Matthias Kaehlcke
This is like commit 0ca87bd5baa6 ("ARM: dts: rockchip: Add pin names
for rk3288-veyron-jerry") and other similar commits, but for the
veyron fievel board (and tiger, which includes the fievel .dtsi).
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
arch/arm/boot/dts/rk3288-veyron-fievel.dts | 214 +++++++++++++++++++++
1 file changed, 214 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-veyron-fievel.dts b/arch/arm/boot/dts/rk3288-veyron-fievel.dts
index 696566f72d30..5c14a8fa6574 100644
--- a/arch/arm/boot/dts/rk3288-veyron-fievel.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-fievel.dts
@@ -198,6 +198,220 @@
pinctrl-0 = <&drv_5v>;
};
+&gpio0 {
+ gpio-line-names = "PMIC_SLEEP_AP",
+ "DDRIO_PWROFF",
+ "DDRIO_RETEN",
+ "TS3A227E_INT_L",
+ "PMIC_INT_L",
+ "PWR_KEY_L",
+ "HUB_USB1_nFALUT",
+ "PHY_PMEB",
+
+ "PHY_INT",
+ "REC_MODE_L",
+ "OTP_OUT",
+ "",
+ "USB_OTG_POWER_EN",
+ "AP_WARM_RESET_H",
+ "USB_OTG_nFALUT",
+ "I2C0_SDA_PMIC",
+
+ "I2C0_SCL_PMIC",
+ "DEVMODE_L",
+ "USB_INT";
+};
+
+&gpio2 {
+ gpio-line-names = "CONFIG0",
+ "CONFIG1",
+ "CONFIG2",
+ "",
+ "",
+ "",
+ "",
+ "CONFIG3",
+
+ "",
+ "EMMC_RST_L",
+ "",
+ "",
+ "BL_PWR_EN",
+ "",
+ "TOUCH_INT",
+ "TOUCH_RST",
+
+ "I2C3_SCL_TP",
+ "I2C3_SDA_TP";
+};
+
+&gpio3 {
+ gpio-line-names = "FLASH0_D0",
+ "FLASH0_D1",
+ "FLASH0_D2",
+ "FLASH0_D3",
+ "FLASH0_D4",
+ "FLASH0_D5",
+ "FLASH0_D6",
+ "FLASH0_D7",
+
+ "VCC5V_GOOD_H",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "",
+
+ "FLASH0_CS2/EMMC_CMD",
+ "",
+ "FLASH0_DQS/EMMC_CLKO",
+ "",
+ "",
+ "",
+ "",
+ "",
+
+ "PHY_TXD2",
+ "PHY_TXD3",
+ "MAC_RXD2",
+ "MAC_RXD3",
+ "PHY_TXD0",
+ "PHY_TXD1",
+ "MAC_RXD0",
+ "MAC_RXD1";
+};
+
+&gpio4 {
+ gpio-line-names = "MAC_MDC",
+ "MAC_RXDV",
+ "MAC_RXER",
+ "MAC_CLK",
+ "PHY_TXEN",
+ "MAC_MDIO",
+ "MAC_RXCLK",
+ "",
+
+ "PHY_RST",
+ "PHY_TXCLK",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "",
+
+ "UART0_RXD",
+ "UART0_TXD",
+ "UART0_CTS_L",
+ "UART0_RTS_L",
+ "SDIO0_D0",
+ "SDIO0_D1",
+ "SDIO0_D2",
+ "SDIO0_D3",
+
+ "SDIO0_CMD",
+ "SDIO0_CLK",
+ "BT_DEV_WAKE",
+ "",
+ "WIFI_ENABLE_H",
+ "BT_ENABLE_L",
+ "WIFI_HOST_WAKE",
+ "BT_HOST_WAKE";
+};
+
+&gpio5 {
+ gpio-line-names = "",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "",
+ "",
+
+ "",
+ "",
+ "",
+ "",
+ "USB_OTG_CTL1",
+ "HUB_USB2_CTL1",
+ "HUB_USB2_PWR_EN",
+ "HUB_USB_ILIM_SEL",
+
+ "USB_OTG_STATUS_L",
+ "HUB_USB1_CTL1",
+ "HUB_USB1_PWR_EN",
+ "VCC50_HDMI_EN";
+};
+
+&gpio6 {
+ gpio-line-names = "I2S0_SCLK",
+ "I2S0_LRCK_RX",
+ "I2S0_LRCK_TX",
+ "I2S0_SDI",
+ "I2S0_SDO0",
+ "HP_DET_H",
+ "",
+ "INT_CODEC",
+
+ "I2S0_CLK",
+ "I2C2_SDA",
+ "I2C2_SCL",
+ "MICDET",
+ "",
+ "",
+ "",
+ "",
+
+ "HUB_USB2_nFALUT",
+ "USB_OTG_ILIM_SEL";
+};
+
+&gpio7 {
+ gpio-line-names = "LCD_BL_PWM",
+ "PWM_LOG",
+ "BL_EN",
+ "PWR_LED1",
+ "TPM_INT_H",
+ "SPK_ON",
+ "FW_WP_AP",
+ "",
+
+ "CPU_NMI",
+ "DVSOK",
+ "",
+ "EDP_HPD",
+ "DVS1",
+ "",
+ "LCD_EN",
+ "DVS2",
+
+ "HDMI_CEC",
+ "I2C4_SDA",
+ "I2C4_SCL",
+ "I2C5_SDA_HDMI",
+ "I2C5_SCL_HDMI",
+ "5V_DRV",
+ "UART2_RXD",
+ "UART2_TXD";
+};
+
+&gpio8 {
+ gpio-line-names = "RAM_ID0",
+ "RAM_ID1",
+ "RAM_ID2",
+ "RAM_ID3",
+ "I2C1_SDA_TPM",
+ "I2C1_SCL_TPM",
+ "SPI2_CLK",
+ "SPI2_CS0",
+
+ "SPI2_RXD",
+ "SPI2_TXD";
+};
+
&pinctrl {
pinctrl-names = "default", "sleep";
pinctrl-0 = <
--
2.22.0.770.g0f2c4a37fd-goog
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Rob Herring @ 2019-08-01 22:59 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-mmc, Ulf Hansson, Mark Rutland, Joel Stanley, Adrian Hunter,
devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-aspeed, linux-kernel@vger.kernel.org, Ryan Chen
In-Reply-To: <20190730062316.32037-2-andrew@aj.id.au>
On Tue, Jul 30, 2019 at 12:23 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the
> SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> data bus if only a single slot is enabled.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>
> ---
> v3:
> * Fix compatible enums
> * Add AST2600 compatibles
> * Describe #address-cells / #size-cells
> ---
> .../devicetree/bindings/mmc/aspeed,sdhci.yaml | 100 ++++++++++++++++++
> 1 file changed, 100 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
>
> diff --git a/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> new file mode 100644
> index 000000000000..dd2a00c59641
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/aspeed,sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED SD/SDIO/eMMC Controller
> +
> +maintainers:
> + - Andrew Jeffery <andrew@aj.id.au>
> + - Ryan Chen <ryanchen.aspeed@gmail.com>
> +
> +description: |+
> + The ASPEED SD/SDIO/eMMC controller exposes two slots implementing the SDIO
> + Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit data bus if
> + only a single slot is enabled.
> +
> + The two slots are supported by a common configuration area. As the SDHCIs for
> + the slots are dependent on the common configuration area, they are described
> + as child nodes.
> +
> +properties:
> + compatible:
> + enum:
> + - aspeed,ast2400-sd-controller
> + - aspeed,ast2500-sd-controller
> + - aspeed,ast2600-sd-controller
> + reg:
> + maxItems: 1
> + description: Common configuration registers
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 1
> + ranges: true
> + clocks:
> + maxItems: 1
> + description: The SD/SDIO controller clock gate
> +
> +patternProperties:
> + "^sdhci@[0-9a-f]+$":
This should probably have:
allOf:
- $ref: mmc-controller.yaml
Another new thing in 5.3. :)
> + type: object
> + properties:
> + compatible:
> + enum:
> + - aspeed,ast2400-sdhci
> + - aspeed,ast2500-sdhci
> + - aspeed,ast2600-sdhci
> + reg:
> + maxItems: 1
> + description: The SDHCI registers
> + clocks:
> + maxItems: 1
> + description: The SD bus clock
> + interrupts:
> + maxItems: 1
> + description: The SD interrupt shared between both slots
> + required:
> + - compatible
> + - reg
> + - clocks
> + - interrupts
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> + - ranges
> + - clocks
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/aspeed-clock.h>
> + sdc@1e740000 {
> + compatible = "aspeed,ast2500-sd-controller";
> + reg = <0x1e740000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x1e740000 0x10000>;
> + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
> +
> + sdhci0: sdhci@100 {
> + compatible = "aspeed,ast2500-sdhci";
> + reg = <0x100 0x100>;
> + interrupts = <26>;
> + sdhci,auto-cmd12;
Not documented. Maybe should be common, but there's only a few users.
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + };
> +
> + sdhci1: sdhci@200 {
> + compatible = "aspeed,ast2500-sdhci";
> + reg = <0x200 0x100>;
> + interrupts = <26>;
> + sdhci,auto-cmd12;
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + };
> + };
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH v3 4/5] RISC-V: Export few kernel symbols
From: Atish Patra @ 2019-08-01 23:04 UTC (permalink / raw)
To: hch@infradead.org
Cc: info@metux.net, palmer@sifive.com, allison@lohutok.net,
aou@eecs.berkeley.edu, linux-kernel@vger.kernel.org,
paul.walmsley@sifive.com, devicetree@vger.kernel.org,
tglx@linutronix.de, daniel.lezcano@linaro.org, Anup Patel,
mark.rutland@arm.com, robh+dt@kernel.org, johan@kernel.org,
gregkh@linuxfoundation.org, tiny.windzz@gmail.com,
gary@garyguo.net, linux-riscv
In-Reply-To: <20190801155633.GA366@infradead.org>
On Thu, 2019-08-01 at 08:56 -0700, Christoph Hellwig wrote:
> On Wed, Jul 31, 2019 at 05:58:42PM -0700, Atish Patra wrote:
> > Export few symbols used by kvm module. Without this, kvm can not
> > be compiled as a module.
>
> Please add this to the kvm series instead as we don't merge exports
> without their users in the same series.
Sure.
Regards,
Atish
^ permalink raw reply
* Re: [PATCH v3 5/5] dt-bindings: Update the riscv,isa string description
From: Atish Patra @ 2019-08-01 23:07 UTC (permalink / raw)
To: robh+dt@kernel.org
Cc: info@metux.net, palmer@sifive.com, allison@lohutok.net,
aou@eecs.berkeley.edu, linux-kernel@vger.kernel.org,
paul.walmsley@sifive.com, devicetree@vger.kernel.org,
tglx@linutronix.de, daniel.lezcano@linaro.org, Anup Patel,
mark.rutland@arm.com, johan@kernel.org, tiny.windzz@gmail.com,
gregkh@linuxfoundation.org, gary@garyguo.net,
linux-riscv@lists.infradead.org
In-Reply-To: <CAL_JsqLqxN1+fvrdD24Ho6s7gB+pGy-0sZaL-jJqkYZ2yC4JEA@mail.gmail.com>
On Thu, 2019-08-01 at 09:50 -0600, Rob Herring wrote:
> On Wed, Jul 31, 2019 at 6:58 PM Atish Patra <atish.patra@wdc.com>
> wrote:
> > Since the RISC-V specification states that ISA description strings
> > are
> > case-insensitive, there's no functional difference between mixed-
> > case,
> > upper-case, and lower-case ISA strings. Thus, to simplify parsing,
> > specify that the letters present in "riscv,isa" must be all
> > lowercase.
> >
> > Suggested-by: Paul Walmsley <paul.walmsley@sifive.com>
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > Documentation/devicetree/bindings/riscv/cpus.yaml | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index c899111aa5e3..4f0acb00185a 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -46,10 +46,12 @@ properties:
> > - rv64imafdc
> > description:
> > Identifies the specific RISC-V instruction set architecture
> > - supported by the hart. These are documented in the RISC-V
> > + supported by the hart. These are documented in the RISC-V
> > User-Level ISA document, available from
> > https://riscv.org/specifications/
> >
> > + Letters in the riscv,isa string must be all lowercase.
> > +
>
> The schemas are case sensitive this looks pretty pointless without
> the
> context of the commit msg. Can you prefix with 'While the
> specification is case insensitive, "
>
Sure. How about this ?
"While the above isa strings in ISA specification are case insensitive,
letters in the riscv,isa string must be all lowercase to simplify
parsing."
> For some background, FDT generally always has been case sensitive too
> (dtc won't merge/override nodes/properties with differing case). It's
> really only some older true OF systems that were case insensitive.
> The
> kernel had a mixture of case sensitive and insensitive comparisons
> somewhat depending on the arch and whether of_prop_cmp/of_node_cmp or
> str*cmp functions were used. There's been a lot of clean-up and now
> most comparisons are case sensitive with only Sparc having some
> deviation.
>
> Rob
--
Regards,
Atish
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 23:19 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <85cd5100-467e-d08e-0ae5-ae57a6de5312@nvidia.com>
On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>
> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>> This patch implements save and restore context for peripheral
>>>>>>>>>>> fixed
>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock ops, and
>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>
>>>>>>>>>>> During system suspend, core power goes off and looses the
>>>>>>>>>>> settings
>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>
>>>>>>>>>>> So during suspend entry clock and reset state of peripherals is
>>>>>>>>>>> saved
>>>>>>>>>>> and on resume they are restored to have clocks back to same
>>>>>>>>>>> rate and
>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>
>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>>> ---
>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>> @@ -60,11 +60,44 @@ tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>> }
>>>>>>>>>>> +static int tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>> clk_hw
>>>>>>>>>>> *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>> +
>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>> + mask;
>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>> + mask;
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>> clk_hw
>>>>>>>>>>> *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>> +
>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>> + else
>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>> +
>>>>>>>>>>> + udelay(2);
>>>>>>>>>>> +
>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> static const struct clk_ops tegra_clk_periph_fixed_ops = {
>>>>>>>>>>> .is_enabled = tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>> .recalc_rate = tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>>>>> + .restore_context = tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>> };
>>>>>>>>>>> struct clk *tegra_clk_register_periph_fixed(const char
>>>>>>>>>>> *name,
>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>> @@ -25,6 +25,8 @@ static DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>> readl_relaxed(gate->clk_base + (gate->regs->rst_reg))
>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>> }
>>>>>>>>>>> +static int clk_periph_gate_save_context(struct clk_hw
>>>>>>>>>>> *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>> +
>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct clk_hw *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>> +
>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>> + else
>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>> +
>>>>>>>>>>> + udelay(5);
>>>>>>>>>>> +
>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>> + else
>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>> + }
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>>>>> };
>>>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const char
>>>>>>>>>>> *name,
>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct clk_hw
>>>>>>>>>>> *hw)
>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>> }
>>>>>>>>>>> +static int clk_periph_save_context(struct clk_hw *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>> +
>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>> +
>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw *hw)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>> +
>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>> +
>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>> Could you please point to where the divider's save_context()
>>>>>>>>>> happens?
>>>>>>>>>> Because I can't see it.
>>>>>>>>> Ah, I now see that there is no need to save the dividers context
>>>>>>>>> because
>>>>>>>>> clk itself has enough info that is needed for the context's
>>>>>>>>> restoring
>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>
>>>>>>>>> Looks like you could also implement a new
>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>> generic helper to get the index instead of storing it manually.
>>>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>
>>>>>>>> All existing drivers are using directly get_parent() from clk_mux
>>>>>>>> which actually gets index from the register read.
>>>>>>>>
>>>>>>>> To have this more generic w.r.t save/restore context point of
>>>>>>>> view,
>>>>>>>> probably instead of implementing new get_parent_index helper, I
>>>>>>>> think
>>>>>>>> its better to implement save_context and restore_context to
>>>>>>>> clk_mux_ops along with creating parent_index field into clk_mux to
>>>>>>>> cache index during set_parent.
>>>>>>>>
>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>> restore_context.
>>>>>>>>
>>>>>>> I hope its ok to add save/restore context to clk_mux_ops to be more
>>>>>>> generic w.r.t save/restore context rather than get_parent_index
>>>>>>> API.
>>>>>>> Please confirm if you agree.
>>>>>> Sounds like a good idea. I see that there is a 'restoring' helper
>>>>>> for
>>>>>> the generic clk_gate, seems something similar could be done for the
>>>>>> clk_mux. And looks like anyway you'll need to associate the parent
>>>>>> clock
>>>>>> with the hw index in order to restore the muxing.
>>>>> by 'restoring' helper for generic clk_gate, are you referring to
>>>>> clk_gate_restore_context API?
>>>> Yes.
>>>>
>>>>> clk_gate_restore_context is API that's any clk drivers can use for
>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>
>>>>> But clk-periph is directly using generic clk_mux ops from clk_mux
>>>>> so I
>>>>> think we should add .restore_context to clk_mux_ops and then during
>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>> I'm not sure whether it will be good for every driver that uses
>>>> generic
>>>> clk_mux ops. Should be more flexible to have a generic helper function
>>>> that any driver could use in order to restore the clock's parent.
>>>>
>>>> The clk-periph restoring also includes case of combining divider and
>>>> parent restoring, so generic helper could be useful in that case as
>>>> well.
>>>>
>>>> It also looks like you could actually use the
>>>> clk_gate_restore_context()
>>>> instead of manually saving the clock's enable-state, couldn't you?
>>> ok for clk_mux, can add generic clk_mux_restore_context API rather than
>>> using restore_context in clk_ops and will invoke that during clk_periph
>>> restore.
>>>
>>>
>>> Reg clk_gate, looks like we cant use generic clk_gate_restore_context
>>> for clk-periph as it calls enable/disable callbacks and
>>> clk_periph_enable/disable in clk-periph-gate also updated refcnt and
>>> depending on that actual enable/disable is set.
>>>
>>> During suspend, peripherals that are already enabled have their
>>> refcnt >
>>> 1, so they dont go thru enable/disable on restore if we use same
>>> enable/disable callback.
>> Looks like you could just decrement the gate's enable_refcnt on
>> save_context, wouldn't that work?
>>
gate->enable_refcnt is within clk-periph-gate which gets updated when
enable/disable callbacks get execute thru clk_core_enable/disable.
But actual enable_count used in clk_gate_restore_context is the one
which gets updated with in the clk core enable/disable functions which
invokes these callbacks. Depending on this enable_count in clk core it
invokes enable/disable.
So, this will cause mismatch if we handle refcnt during save/restore of
tegra_clk_periph_gate_ops and also enable/disable thru this
clk_gate_restore_context is based on enable_count from clk core.
>>> Also to align exact reset state along with CLK (like for case where CLK
>>> is enabled but peripheral might be in reset state), implemented
>>> save/restore in tegra specific tegra_clk_periph_gate_ops
>> I'm wondering whether instead of saving/restoring reset-state of every
>> clock, you could simply save/restore the whole RST_DEV_x_SET register.
>> Couldn't you?
> Thats what I was doing in first version of patch. But later as we
> moved to use clk_save_context and clk_restore_context, peripheral
> clk_hw RST & CLK enables happen thru its corresponding save/restore
> after source restore
Also, to align both CLK & RST to the exact state of register, doing
save/restore in tegra_clk_periph_gate_ops and invoking this after source
restore for peripheral clock, seems cleaner to avoid any
misconfiguration b/w rst & clk settings.
^ 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