* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23 1:08 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: <0b3d08ea-4633-8a54-ba66-c3f3146a1ece@gmail.com>
23.07.2019 3:58, Dmitry Osipenko пишет:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch implements PMC wakeup sequence for Tegra210 and defines
>> common used RTC alarm wake event.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 111 insertions(+)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 91c84d0e66ae..c556f38874e1 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -57,6 +57,12 @@
>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>> #define PMC_CNTRL_MAIN_RST BIT(4)
>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
Please follow the TRM's bits naming.
PMC_CNTRL_LATCHWAKE_EN
>> +#define PMC_WAKE_MASK 0x0c
>> +#define PMC_WAKE_LEVEL 0x10
>> +#define PMC_WAKE_STATUS 0x14
>> +#define PMC_SW_WAKE_STATUS 0x18
>>
>> #define DPD_SAMPLE 0x020
>> #define DPD_SAMPLE_ENABLE BIT(0)
>> @@ -87,6 +93,11 @@
>>
>> #define PMC_SCRATCH41 0x140
>>
>> +#define PMC_WAKE2_MASK 0x160
>> +#define PMC_WAKE2_LEVEL 0x164
>> +#define PMC_WAKE2_STATUS 0x168
>> +#define PMC_SW_WAKE2_STATUS 0x16c
>> +
>> #define PMC_SENSOR_CTRL 0x1b0
>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>> .alloc = tegra_pmc_irq_alloc,
>> };
>>
>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>> +{
>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>> + unsigned int offset, bit;
>> + u32 value;
>> +
>> + if (data->hwirq == ULONG_MAX)
>> + return 0;
>> +
>> + offset = data->hwirq / 32;
>> + bit = data->hwirq % 32;
>> +
>> + /*
>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>> + * that would not make it into wakeup event register during LP0 exit.
>> + */
>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>> + udelay(120);
>
> Why it takes so much time to latch the values? Shouldn't some status-bit
> be polled for the completion of latching?
>
> Is this register-write really getting buffered in the PMC?
>
>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>> + udelay(120);
>
> 120 usecs to remove latching, really?
>
>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>> +
>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>> +
>> + /* enable PMC wake */
>> + if (data->hwirq >= 32)
>> + offset = PMC_WAKE2_MASK;
>> + else
>> + offset = PMC_WAKE_MASK;
>> +
>> + value = tegra_pmc_readl(pmc, offset);
>> +
>> + if (on)
>> + value |= 1 << bit;
>> + else
>> + value &= ~(1 << bit);
>> +
>> + tegra_pmc_writel(pmc, value, offset);
>
> Why the latching is done *before* writing into the WAKE registers? What
> it is latching then?
I'm looking at the TRM doc and it says that latching should be done
*after* writing to the WAKE_MASK / LEVEL registers.
Secondly it says that it's enough to do:
value = tegra_pmc_readl(pmc, PMC_CNTRL);
value |= PMC_CNTRL_LATCH_WAKEUPS;
tegra_pmc_writel(pmc, value, PMC_CNTRL);
in order to latch. There is no need for the delay and to remove the
"LATCHWAKE_EN" bit, it should be a oneshot action.
>> + return 0;
>> +}
>> +
>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>> {
>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>> return 0;
>> }
>>
>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>> +{
>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>> + unsigned int offset, bit;
>> + u32 value;
>> +
>> + if (data->hwirq == ULONG_MAX)
>> + return 0;
>> +
>> + offset = data->hwirq / 32;
>> + bit = data->hwirq % 32;
>> +
>> + if (data->hwirq >= 32)
>> + offset = PMC_WAKE2_LEVEL;
>> + else
>> + offset = PMC_WAKE_LEVEL;
>> +
>> + value = tegra_pmc_readl(pmc, offset);
>> +
>> + switch (type) {
>> + case IRQ_TYPE_EDGE_RISING:
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + value |= 1 << bit;
>> + break;
>> +
>> + case IRQ_TYPE_EDGE_FALLING:
>> + case IRQ_TYPE_LEVEL_LOW:
>> + value &= ~(1 << bit);
>> + break;
>> +
>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>> + value ^= 1 << bit;
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + tegra_pmc_writel(pmc, value, offset);
>
> Shouldn't the WAKE_LEVEL be latched as well?
>
>> + return 0;
>> +}
>> +
>> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>> {
>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>> @@ -2540,6 +2643,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>> };
>>
>> +static const struct tegra_wake_event tegra210_wake_events[] = {
>> + TEGRA_WAKE_IRQ("rtc", 16, 2),
>> +};
>> +
>> static const struct tegra_pmc_soc tegra210_pmc_soc = {
>> .num_powergates = ARRAY_SIZE(tegra210_powergates),
>> .powergates = tegra210_powergates,
>> @@ -2557,10 +2664,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>> .regs = &tegra20_pmc_regs,
>> .init = tegra20_pmc_init,
>> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
>> + .irq_set_wake = tegra210_pmc_irq_set_wake,
>> + .irq_set_type = tegra210_pmc_irq_set_type,
>> .reset_sources = tegra210_reset_sources,
>> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>> .reset_levels = NULL,
>> .num_reset_levels = 0,
>> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
>> + .wake_events = tegra210_wake_events,
>> };
>>
>> #define TEGRA186_IO_PAD_TABLE(_pad) \
>>
>
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23 1:41 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: <ca32c2d8-d752-3ecd-3a3f-232366730c7b@gmail.com>
23.07.2019 4:08, Dmitry Osipenko пишет:
> 23.07.2019 3:58, Dmitry Osipenko пишет:
>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>> common used RTC alarm wake event.
>>>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 111 insertions(+)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index 91c84d0e66ae..c556f38874e1 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -57,6 +57,12 @@
>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>
> Please follow the TRM's bits naming.
>
> PMC_CNTRL_LATCHWAKE_EN
>
>>> +#define PMC_WAKE_MASK 0x0c
>>> +#define PMC_WAKE_LEVEL 0x10
>>> +#define PMC_WAKE_STATUS 0x14
>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>
>>> #define DPD_SAMPLE 0x020
>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>> @@ -87,6 +93,11 @@
>>>
>>> #define PMC_SCRATCH41 0x140
>>>
>>> +#define PMC_WAKE2_MASK 0x160
>>> +#define PMC_WAKE2_LEVEL 0x164
>>> +#define PMC_WAKE2_STATUS 0x168
>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>> +
>>> #define PMC_SENSOR_CTRL 0x1b0
>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>> .alloc = tegra_pmc_irq_alloc,
>>> };
>>>
>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>> +{
>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>> + unsigned int offset, bit;
>>> + u32 value;
>>> +
>>> + if (data->hwirq == ULONG_MAX)
>>> + return 0;
>>> +
>>> + offset = data->hwirq / 32;
>>> + bit = data->hwirq % 32;
>>> +
>>> + /*
>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>> + * that would not make it into wakeup event register during LP0 exit.
>>> + */
>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>> + udelay(120);
>>
>> Why it takes so much time to latch the values? Shouldn't some status-bit
>> be polled for the completion of latching?
>>
>> Is this register-write really getting buffered in the PMC?
>>
>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>> + udelay(120);
>>
>> 120 usecs to remove latching, really?
>>
>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>> +
>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>> +
>>> + /* enable PMC wake */
>>> + if (data->hwirq >= 32)
>>> + offset = PMC_WAKE2_MASK;
>>> + else
>>> + offset = PMC_WAKE_MASK;
>>> +
>>> + value = tegra_pmc_readl(pmc, offset);
>>> +
>>> + if (on)
>>> + value |= 1 << bit;
>>> + else
>>> + value &= ~(1 << bit);
>>> +
>>> + tegra_pmc_writel(pmc, value, offset);
>>
>> Why the latching is done *before* writing into the WAKE registers? What
>> it is latching then?
>
> I'm looking at the TRM doc and it says that latching should be done
> *after* writing to the WAKE_MASK / LEVEL registers.
>
> Secondly it says that it's enough to do:
>
> value = tegra_pmc_readl(pmc, PMC_CNTRL);
> value |= PMC_CNTRL_LATCH_WAKEUPS;
> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>
> in order to latch. There is no need for the delay and to remove the
> "LATCHWAKE_EN" bit, it should be a oneshot action.
Although, no. TRM says "stops latching on transition from 1
to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
Have you tested this code at all? I'm wondering how it happens to work
without a proper latching.
>>> + return 0;
>>> +}
>>> +
>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>> {
>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>> return 0;
>>> }
>>>
>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>> +{
>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>> + unsigned int offset, bit;
>>> + u32 value;
>>> +
>>> + if (data->hwirq == ULONG_MAX)
>>> + return 0;
>>> +
>>> + offset = data->hwirq / 32;
>>> + bit = data->hwirq % 32;
>>> +
>>> + if (data->hwirq >= 32)
>>> + offset = PMC_WAKE2_LEVEL;
>>> + else
>>> + offset = PMC_WAKE_LEVEL;
>>> +
>>> + value = tegra_pmc_readl(pmc, offset);
>>> +
>>> + switch (type) {
>>> + case IRQ_TYPE_EDGE_RISING:
>>> + case IRQ_TYPE_LEVEL_HIGH:
>>> + value |= 1 << bit;
>>> + break;
>>> +
>>> + case IRQ_TYPE_EDGE_FALLING:
>>> + case IRQ_TYPE_LEVEL_LOW:
>>> + value &= ~(1 << bit);
>>> + break;
>>> +
>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>> + value ^= 1 << bit;
>>> + break;
>>> +
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + tegra_pmc_writel(pmc, value, offset);
>>
>> Shouldn't the WAKE_LEVEL be latched as well?
>>
>>> + return 0;
>>> +}
>>> +
>>> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>> {
>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>> @@ -2540,6 +2643,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>>> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>>> };
>>>
>>> +static const struct tegra_wake_event tegra210_wake_events[] = {
>>> + TEGRA_WAKE_IRQ("rtc", 16, 2),
>>> +};
>>> +
>>> static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>> .num_powergates = ARRAY_SIZE(tegra210_powergates),
>>> .powergates = tegra210_powergates,
>>> @@ -2557,10 +2664,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>> .regs = &tegra20_pmc_regs,
>>> .init = tegra20_pmc_init,
>>> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
>>> + .irq_set_wake = tegra210_pmc_irq_set_wake,
>>> + .irq_set_type = tegra210_pmc_irq_set_type,
>>> .reset_sources = tegra210_reset_sources,
>>> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>>> .reset_levels = NULL,
>>> .num_reset_levels = 0,
>>> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
>>> + .wake_events = tegra210_wake_events,
>>> };
>>>
>>> #define TEGRA186_IO_PAD_TABLE(_pad) \
>>>
>>
>
^ permalink raw reply
* Re: [PATCH v3 2/5] opp: core: add regulators enable and disable
From: Viresh Kumar @ 2019-07-23 1:48 UTC (permalink / raw)
To: k.konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190719150535.15501-3-k.konieczny@partner.samsung.com>
On 19-07-19, 17:05, k.konieczny@partner.samsung.com wrote:
> Add enable regulators to dev_pm_opp_set_regulators() and disable
> regulators to dev_pm_opp_put_regulators(). Even if bootloader
> leaves regulators enabled, they should be enabled in kernel in
> order to increase the reference count.
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> Changes in v3:
> - corrected error path in enable
> - improved commit message
> Changes in v2:
> - move regulator enable and disable into loop
> ---
> drivers/opp/core.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 0e7703fe733f..a8a480cdabab 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1570,6 +1570,12 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
> goto free_regulators;
> }
>
> + ret = regulator_enable(reg);
> + if (ret < 0) {
> + regulator_put(reg);
> + goto free_regulators;
> + }
> +
> opp_table->regulators[i] = reg;
> }
>
> @@ -1583,8 +1589,10 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
> return opp_table;
>
> free_regulators:
> - while (i != 0)
> - regulator_put(opp_table->regulators[--i]);
> + while (i--) {
> + regulator_disable(opp_table->regulators[i]);
> + regulator_put(opp_table->regulators[i]);
> + }
>
> kfree(opp_table->regulators);
> opp_table->regulators = NULL;
> @@ -1610,8 +1618,10 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
> /* Make sure there are no concurrent readers while updating opp_table */
> WARN_ON(!list_empty(&opp_table->opp_list));
>
> - for (i = opp_table->regulator_count - 1; i >= 0; i--)
> + for (i = opp_table->regulator_count - 1; i >= 0; i--) {
> + regulator_disable(opp_table->regulators[i]);
> regulator_put(opp_table->regulators[i]);
> + }
>
> _free_set_opp_data(opp_table);
Applied. Thanks.
--
viresh
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Sowjanya Komatineni @ 2019-07-23 1:52 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: <b575ca93-9f34-b07a-1234-ef1ea2a6ddee@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7769 bytes --]
On 7/22/19 6:41 PM, Dmitry Osipenko wrote:
> 23.07.2019 4:08, Dmitry Osipenko пишет:
>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>> common used RTC alarm wake event.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 111 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>> --- a/drivers/soc/tegra/pmc.c
>>>> +++ b/drivers/soc/tegra/pmc.c
>>>> @@ -57,6 +57,12 @@
>>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>> Please follow the TRM's bits naming.
>>
>> PMC_CNTRL_LATCHWAKE_EN
>>
>>>> +#define PMC_WAKE_MASK 0x0c
>>>> +#define PMC_WAKE_LEVEL 0x10
>>>> +#define PMC_WAKE_STATUS 0x14
>>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>>
>>>> #define DPD_SAMPLE 0x020
>>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>>> @@ -87,6 +93,11 @@
>>>>
>>>> #define PMC_SCRATCH41 0x140
>>>>
>>>> +#define PMC_WAKE2_MASK 0x160
>>>> +#define PMC_WAKE2_LEVEL 0x164
>>>> +#define PMC_WAKE2_STATUS 0x168
>>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>>> +
>>>> #define PMC_SENSOR_CTRL 0x1b0
>>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>>> .alloc = tegra_pmc_irq_alloc,
>>>> };
>>>>
>>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> +{
>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> + unsigned int offset, bit;
>>>> + u32 value;
>>>> +
>>>> + if (data->hwirq == ULONG_MAX)
>>>> + return 0;
>>>> +
>>>> + offset = data->hwirq / 32;
>>>> + bit = data->hwirq % 32;
>>>> +
>>>> + /*
>>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>> + * that would not make it into wakeup event register during LP0 exit.
>>>> + */
>>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>> + udelay(120);
>>> Why it takes so much time to latch the values? Shouldn't some status-bit
>>> be polled for the completion of latching?
>>>
>>> Is this register-write really getting buffered in the PMC?
>>>
>>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>> + udelay(120);
>>> 120 usecs to remove latching, really?
>>>
>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>> +
>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>> +
>>>> + /* enable PMC wake */
>>>> + if (data->hwirq >= 32)
>>>> + offset = PMC_WAKE2_MASK;
>>>> + else
>>>> + offset = PMC_WAKE_MASK;
>>>> +
>>>> + value = tegra_pmc_readl(pmc, offset);
>>>> +
>>>> + if (on)
>>>> + value |= 1 << bit;
>>>> + else
>>>> + value &= ~(1 << bit);
>>>> +
>>>> + tegra_pmc_writel(pmc, value, offset);
>>> Why the latching is done *before* writing into the WAKE registers? What
>>> it is latching then?
>> I'm looking at the TRM doc and it says that latching should be done
>> *after* writing to the WAKE_MASK / LEVEL registers.
>>
>> Secondly it says that it's enough to do:
>>
>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>
>> in order to latch. There is no need for the delay and to remove the
>> "LATCHWAKE_EN" bit, it should be a oneshot action.
> Although, no. TRM says "stops latching on transition from 1
> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>
> Have you tested this code at all? I'm wondering how it happens to work
> without a proper latching.
Yes, ofcourse its tested and this sequence to do transition is
recommendation from Tegra designer.
Will check if TRM doesn't have update properly or will re-confirm
internally on delay time...
On any of the wake event PMC wakeup happens and WAKE_STATUS register
will have bits set for all events that triggered wake.
After wakeup PMC doesn't update SW_WAKE_STATUS register as per PMC design.
SW latch register added in design helps to provide a way to capture
those events that happen right during wakeup time and didnt make it to
SW_WAKE_STATUS register.
So before next suspend entry, latching all prior wake events into SW
WAKE_STATUS and then clearing them.
LATCHWAKE_EN - When set, enables latching and stops latching on
transition from 1 to 0
There is recommendation of min 120uSec for this transition to stop
latching. Will double-check why 120uSec
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> {
>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> return 0;
>>>> }
>>>>
>>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>> +{
>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> + unsigned int offset, bit;
>>>> + u32 value;
>>>> +
>>>> + if (data->hwirq == ULONG_MAX)
>>>> + return 0;
>>>> +
>>>> + offset = data->hwirq / 32;
>>>> + bit = data->hwirq % 32;
>>>> +
>>>> + if (data->hwirq >= 32)
>>>> + offset = PMC_WAKE2_LEVEL;
>>>> + else
>>>> + offset = PMC_WAKE_LEVEL;
>>>> +
>>>> + value = tegra_pmc_readl(pmc, offset);
>>>> +
>>>> + switch (type) {
>>>> + case IRQ_TYPE_EDGE_RISING:
>>>> + case IRQ_TYPE_LEVEL_HIGH:
>>>> + value |= 1 << bit;
>>>> + break;
>>>> +
>>>> + case IRQ_TYPE_EDGE_FALLING:
>>>> + case IRQ_TYPE_LEVEL_LOW:
>>>> + value &= ~(1 << bit);
>>>> + break;
>>>> +
>>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>>> + value ^= 1 << bit;
>>>> + break;
>>>> +
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + tegra_pmc_writel(pmc, value, offset);
>>> Shouldn't the WAKE_LEVEL be latched as well?
WAKE_LEVELs dont need any latch as they are the levels SW sets for wake
trigger and they are not status
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>> {
>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> @@ -2540,6 +2643,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>>>> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>>>> };
>>>>
>>>> +static const struct tegra_wake_event tegra210_wake_events[] = {
>>>> + TEGRA_WAKE_IRQ("rtc", 16, 2),
>>>> +};
>>>> +
>>>> static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>> .num_powergates = ARRAY_SIZE(tegra210_powergates),
>>>> .powergates = tegra210_powergates,
>>>> @@ -2557,10 +2664,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>> .regs = &tegra20_pmc_regs,
>>>> .init = tegra20_pmc_init,
>>>> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
>>>> + .irq_set_wake = tegra210_pmc_irq_set_wake,
>>>> + .irq_set_type = tegra210_pmc_irq_set_type,
>>>> .reset_sources = tegra210_reset_sources,
>>>> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>>>> .reset_levels = NULL,
>>>> .num_reset_levels = 0,
>>>> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
>>>> + .wake_events = tegra210_wake_events,
>>>> };
>>>>
>>>> #define TEGRA186_IO_PAD_TABLE(_pad) \
>>>>
[-- Attachment #2: Type: text/html, Size: 9805 bytes --]
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23 1:52 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: <b575ca93-9f34-b07a-1234-ef1ea2a6ddee@gmail.com>
23.07.2019 4:41, Dmitry Osipenko пишет:
> 23.07.2019 4:08, Dmitry Osipenko пишет:
>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>> common used RTC alarm wake event.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 111 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>> --- a/drivers/soc/tegra/pmc.c
>>>> +++ b/drivers/soc/tegra/pmc.c
>>>> @@ -57,6 +57,12 @@
>>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>>
>> Please follow the TRM's bits naming.
>>
>> PMC_CNTRL_LATCHWAKE_EN
>>
>>>> +#define PMC_WAKE_MASK 0x0c
>>>> +#define PMC_WAKE_LEVEL 0x10
>>>> +#define PMC_WAKE_STATUS 0x14
>>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>>
>>>> #define DPD_SAMPLE 0x020
>>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>>> @@ -87,6 +93,11 @@
>>>>
>>>> #define PMC_SCRATCH41 0x140
>>>>
>>>> +#define PMC_WAKE2_MASK 0x160
>>>> +#define PMC_WAKE2_LEVEL 0x164
>>>> +#define PMC_WAKE2_STATUS 0x168
>>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>>> +
>>>> #define PMC_SENSOR_CTRL 0x1b0
>>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>>> .alloc = tegra_pmc_irq_alloc,
>>>> };
>>>>
>>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> +{
>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> + unsigned int offset, bit;
>>>> + u32 value;
>>>> +
>>>> + if (data->hwirq == ULONG_MAX)
>>>> + return 0;
>>>> +
>>>> + offset = data->hwirq / 32;
>>>> + bit = data->hwirq % 32;
>>>> +
>>>> + /*
>>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>> + * that would not make it into wakeup event register during LP0 exit.
>>>> + */
>>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>> + udelay(120);
>>>
>>> Why it takes so much time to latch the values? Shouldn't some status-bit
>>> be polled for the completion of latching?
>>>
>>> Is this register-write really getting buffered in the PMC?
>>>
>>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>> + udelay(120);
>>>
>>> 120 usecs to remove latching, really?
>>>
>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>> +
>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>> +
>>>> + /* enable PMC wake */
>>>> + if (data->hwirq >= 32)
>>>> + offset = PMC_WAKE2_MASK;
>>>> + else
>>>> + offset = PMC_WAKE_MASK;
>>>> +
>>>> + value = tegra_pmc_readl(pmc, offset);
>>>> +
>>>> + if (on)
>>>> + value |= 1 << bit;
>>>> + else
>>>> + value &= ~(1 << bit);
>>>> +
>>>> + tegra_pmc_writel(pmc, value, offset);
>>>
>>> Why the latching is done *before* writing into the WAKE registers? What
>>> it is latching then?
>>
>> I'm looking at the TRM doc and it says that latching should be done
>> *after* writing to the WAKE_MASK / LEVEL registers.
>>
>> Secondly it says that it's enough to do:
>>
>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>
>> in order to latch. There is no need for the delay and to remove the
>> "LATCHWAKE_EN" bit, it should be a oneshot action.
>
> Although, no. TRM says "stops latching on transition from 1
> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>
> Have you tested this code at all? I'm wondering how it happens to work
> without a proper latching.
Okay, I re-read the TRM and apparently "latching" just means storing of
WAKE-event bit in the WAKE-status register if latching is enabled. Hence
the PMC_CNTRL_LATCHWAKE_EN should be enabled in tegra_pmc_suspend() and
unset in tegra_pmc_resume().
Also, apparently, on resume from suspend the interrupt should be
re-triggered in accordance to the WAKE-status, then the WAKE-status need
to be cleared.
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> {
>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>> return 0;
>>>> }
>>>>
>>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>> +{
>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> + unsigned int offset, bit;
>>>> + u32 value;
>>>> +
>>>> + if (data->hwirq == ULONG_MAX)
>>>> + return 0;
>>>> +
>>>> + offset = data->hwirq / 32;
>>>> + bit = data->hwirq % 32;
>>>> +
>>>> + if (data->hwirq >= 32)
>>>> + offset = PMC_WAKE2_LEVEL;
>>>> + else
>>>> + offset = PMC_WAKE_LEVEL;
>>>> +
>>>> + value = tegra_pmc_readl(pmc, offset);
>>>> +
>>>> + switch (type) {
>>>> + case IRQ_TYPE_EDGE_RISING:
>>>> + case IRQ_TYPE_LEVEL_HIGH:
>>>> + value |= 1 << bit;
>>>> + break;
>>>> +
>>>> + case IRQ_TYPE_EDGE_FALLING:
>>>> + case IRQ_TYPE_LEVEL_LOW:
>>>> + value &= ~(1 << bit);
>>>> + break;
>>>> +
>>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>>> + value ^= 1 << bit;
>>>> + break;
>>>> +
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + tegra_pmc_writel(pmc, value, offset);
>>>
>>> Shouldn't the WAKE_LEVEL be latched as well?
>>>
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>> {
>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>> @@ -2540,6 +2643,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>>>> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>>>> };
>>>>
>>>> +static const struct tegra_wake_event tegra210_wake_events[] = {
>>>> + TEGRA_WAKE_IRQ("rtc", 16, 2),
>>>> +};
>>>> +
>>>> static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>> .num_powergates = ARRAY_SIZE(tegra210_powergates),
>>>> .powergates = tegra210_powergates,
>>>> @@ -2557,10 +2664,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>> .regs = &tegra20_pmc_regs,
>>>> .init = tegra20_pmc_init,
>>>> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
>>>> + .irq_set_wake = tegra210_pmc_irq_set_wake,
>>>> + .irq_set_type = tegra210_pmc_irq_set_type,
>>>> .reset_sources = tegra210_reset_sources,
>>>> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>>>> .reset_levels = NULL,
>>>> .num_reset_levels = 0,
>>>> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
>>>> + .wake_events = tegra210_wake_events,
>>>> };
>>>>
>>>> #define TEGRA186_IO_PAD_TABLE(_pad) \
>>>>
>>>
>>
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: hwmon: Add binding for pxe1610
From: Guenter Roeck @ 2019-07-23 1:56 UTC (permalink / raw)
To: Vijay Khemka
Cc: Jean Delvare, Rob Herring, Mark Rutland, Joel Stanley,
Andrew Jeffery, linux-hwmon@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-aspeed@lists.ozlabs.org, openbmc @ lists . ozlabs . org,
Sai Dasari
In-Reply-To: <6E2B35D8-B538-4C96-B289-27A87ECD74DB@fb.com>
On 7/22/19 5:12 PM, Vijay Khemka wrote:
>
>
> On 7/22/19, 1:06 PM, "Guenter Roeck" <groeck7@gmail.com on behalf of linux@roeck-us.net> wrote:
>
> On Mon, Jul 22, 2019 at 12:24:48PM -0700, Vijay Khemka wrote:
> > Added new DT binding document for Infineon PXE1610 devices.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > .../devicetree/bindings/hwmon/pxe1610.txt | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/pxe1610.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/pxe1610.txt b/Documentation/devicetree/bindings/hwmon/pxe1610.txt
> > new file mode 100644
> > index 000000000000..635daf4955db
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pxe1610.txt
> > @@ -0,0 +1,15 @@
> > +pxe1610 properties
> > +
> > +Required properties:
> > +- compatible: Must be one of the following:
> > + - "infineon,pxe1610" for pxe1610
> > + - "infineon,pxe1110" for pxe1610
> > + - "infineon,pxm1310" for pxm1310
> > +- reg: I2C address
> > +
> > +Example:
> > +
> > +vr@48 {
> > + compatible = "infineon,pxe1610";
> > + reg = <0x48>;
> > +};
>
> Wouldn't it be better to add this to
> ./Documentation/devicetree/bindings/trivial-devices.txt ?
> Sure, I didn't know about this file. I will add and send another patch. It is
> Documentation/devicetree/bindings/trivial-devices.yaml. How do I abandon
> this patch or just leave it.
>
When you send v2, just add the device to the trivial-devices file instead
and describe the differences to v1 (ie this patch).
Guenter
^ permalink raw reply
* Re: [PATCH 0/5] Add support for WD MyCloud EX2 Ultra (+ versatile UART-based restart/poweroff drivers)
From: Andrew Lunn @ 2019-07-23 1:56 UTC (permalink / raw)
To: Evgeny Kolesnikov
Cc: Mark Rutland, Jason Cooper, linux-pm, Gregory Clement,
Sebastian Reichel, linux-kernel, devicetree, Rob Herring,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <cover.1563822216.git.evgenyz@gmail.com>
On Mon, Jul 22, 2019 at 09:53:00PM +0200, Evgeny Kolesnikov wrote:
> This patchset consists of the DTS, which describes the WD MyCloud EX2 Ultra device,
> 'poweroff' and 'resert' drivers for power-managing MCUs connected to a board via UART
> (these drivers are more versatile than qnap-poweroff and could be used as a substitude),
> and DT bindings for these drivers.
>
> The difference between uart-poweroff and qnap-poweroff is small, but important:
> uart-poweroff is able to send to an MCU a command of arbitrary length, and the command
> itself is defined in a DTS file for a specific device/board, thus making this driver
> applicable to wider range of devices.
Hi Evgeny
There is a lot of replicated code here, and in the original
qnap-poweroff.c driver. Please consolidate it by extending the current
driver. It should be easy to add a new compatible string, and turn
power_off_cfg.cmd into an array.
Andrew
^ permalink raw reply
* Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
From: Viresh Kumar @ 2019-07-23 1:56 UTC (permalink / raw)
To: Niklas Cassel
Cc: Andy Gross, linux-arm-msm, jorge.ramirez-ortiz, sboyd, vireshk,
bjorn.andersson, ulf.hansson, Rob Herring, Mark Rutland,
devicetree, linux-kernel
In-Reply-To: <20190719154558.GA32518@centauri>
On 19-07-19, 17:45, Niklas Cassel wrote:
> Hello Viresh,
>
> Could you please have a look at the last two patches here:
> https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz
There is no sane way of providing review comments with a link to the
git tree :)
I still had a look and I see that you don't search for max frequency
but just any OPP that has required-opps set to the level u want. Also,
can't there be multiple phandles in required-opps in your case ?
> If you like my proposal then I could send out the first patch (the one to
> OPP core) as a real patch (with an improved commit message), and
> incorporate the second patch into my CPR patch series when I send out a V2.
Send them both in your series only, otherwise the first one is useless
anyway.
--
viresh
^ permalink raw reply
* Re: [PATCH v2 0/9] Exynos Adaptive Supply Voltage support
From: Viresh Kumar @ 2019-07-23 2:04 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: krzk, robh+dt, vireshk, devicetree, kgene, pankaj.dubey,
linux-samsung-soc, linux-arm-kernel, linux-kernel, linux-pm,
b.zolnierkie, m.szyprowski
In-Reply-To: <20190718143044.25066-1-s.nawrocki@samsung.com>
On 18-07-19, 16:30, Sylwester Nawrocki wrote:
> This is second iteration of patch series adding ASV (Adaptive Supply
> Voltage) support for Exynos SoCs. The first one can be found at:
> https://lore.kernel.org/lkml/20190404171735.12815-1-s.nawrocki@samsung.com
>
> The main changes comparing to the first (RFC) version are:
> - moving ASV data tables from DT to the driver,
> - converting the chipid and the ASV drivers to use regmap,
> - converting the ASV driver to proper platform driver.
>
> I tried the opp-supported-hw bitmask approach as in the Qualcomm CPUFreq
> DT bindings but it resulted in too many OPPs and DT nodes, around 200
> per CPU cluster. So the ASV OPP tables are now in the ASV driver, as in
> downstream kernels.
Hmm. Can you explain why do you have so many OPPs? How many
frequencies do you actually support per cluster and what all varies
per frequency based on hw ? How many hw version do u have ?
I am asking as the OPP core can be improved to support your case if
possible. But I need to understand the problem first.
--
viresh
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23 2:10 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: <66535c01-7079-0192-c992-c25a4d7cdbb9@gmail.com>
23.07.2019 4:52, Dmitry Osipenko пишет:
> 23.07.2019 4:41, Dmitry Osipenko пишет:
>> 23.07.2019 4:08, Dmitry Osipenko пишет:
>>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>>> common used RTC alarm wake event.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 111 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>> @@ -57,6 +57,12 @@
>>>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>>>
>>> Please follow the TRM's bits naming.
>>>
>>> PMC_CNTRL_LATCHWAKE_EN
>>>
>>>>> +#define PMC_WAKE_MASK 0x0c
>>>>> +#define PMC_WAKE_LEVEL 0x10
>>>>> +#define PMC_WAKE_STATUS 0x14
>>>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>>>
>>>>> #define DPD_SAMPLE 0x020
>>>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>>>> @@ -87,6 +93,11 @@
>>>>>
>>>>> #define PMC_SCRATCH41 0x140
>>>>>
>>>>> +#define PMC_WAKE2_MASK 0x160
>>>>> +#define PMC_WAKE2_LEVEL 0x164
>>>>> +#define PMC_WAKE2_STATUS 0x168
>>>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>>>> +
>>>>> #define PMC_SENSOR_CTRL 0x1b0
>>>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>>>> .alloc = tegra_pmc_irq_alloc,
>>>>> };
>>>>>
>>>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> +{
>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> + unsigned int offset, bit;
>>>>> + u32 value;
>>>>> +
>>>>> + if (data->hwirq == ULONG_MAX)
>>>>> + return 0;
>>>>> +
>>>>> + offset = data->hwirq / 32;
>>>>> + bit = data->hwirq % 32;
>>>>> +
>>>>> + /*
>>>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>>> + * that would not make it into wakeup event register during LP0 exit.
>>>>> + */
>>>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>> + udelay(120);
>>>>
>>>> Why it takes so much time to latch the values? Shouldn't some status-bit
>>>> be polled for the completion of latching?
>>>>
>>>> Is this register-write really getting buffered in the PMC?
>>>>
>>>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>> + udelay(120);
>>>>
>>>> 120 usecs to remove latching, really?
>>>>
>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>>> +
>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>>> +
>>>>> + /* enable PMC wake */
>>>>> + if (data->hwirq >= 32)
>>>>> + offset = PMC_WAKE2_MASK;
>>>>> + else
>>>>> + offset = PMC_WAKE_MASK;
>>>>> +
>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>> +
>>>>> + if (on)
>>>>> + value |= 1 << bit;
>>>>> + else
>>>>> + value &= ~(1 << bit);
>>>>> +
>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>>
>>>> Why the latching is done *before* writing into the WAKE registers? What
>>>> it is latching then?
>>>
>>> I'm looking at the TRM doc and it says that latching should be done
>>> *after* writing to the WAKE_MASK / LEVEL registers.
>>>
>>> Secondly it says that it's enough to do:
>>>
>>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>
>>> in order to latch. There is no need for the delay and to remove the
>>> "LATCHWAKE_EN" bit, it should be a oneshot action.
>>
>> Although, no. TRM says "stops latching on transition from 1
>> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>>
>> Have you tested this code at all? I'm wondering how it happens to work
>> without a proper latching.
>
> Okay, I re-read the TRM and apparently "latching" just means storing of
> WAKE-event bit in the WAKE-status register if latching is enabled. Hence
> the PMC_CNTRL_LATCHWAKE_EN should be enabled in tegra_pmc_suspend() and
> unset in tegra_pmc_resume().
>
> Also, apparently, on resume from suspend the interrupt should be
> re-triggered in accordance to the WAKE-status, then the WAKE-status need
> to be cleared.
I'm now also recalling that downstream kernel had some problems in
regards to missing power-button presses on resume from suspend because
input driver reads the GPIO-key state in order to determine the press
status and the GPIO state in already unset at the time when input driver
resumes. Hence it happened sometime that after pressing power button,
device waked up from LP0 and then immediately went into suspend (due to
android's wakelocks).
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> {
>>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>>> +{
>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> + unsigned int offset, bit;
>>>>> + u32 value;
>>>>> +
>>>>> + if (data->hwirq == ULONG_MAX)
>>>>> + return 0;
>>>>> +
>>>>> + offset = data->hwirq / 32;
>>>>> + bit = data->hwirq % 32;
>>>>> +
>>>>> + if (data->hwirq >= 32)
>>>>> + offset = PMC_WAKE2_LEVEL;
>>>>> + else
>>>>> + offset = PMC_WAKE_LEVEL;
>>>>> +
>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>> +
>>>>> + switch (type) {
>>>>> + case IRQ_TYPE_EDGE_RISING:
>>>>> + case IRQ_TYPE_LEVEL_HIGH:
>>>>> + value |= 1 << bit;
>>>>> + break;
>>>>> +
>>>>> + case IRQ_TYPE_EDGE_FALLING:
>>>>> + case IRQ_TYPE_LEVEL_LOW:
>>>>> + value &= ~(1 << bit);
>>>>> + break;
>>>>> +
>>>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>>>> + value ^= 1 << bit;
>>>>> + break;
>>>>> +
>>>>> + default:
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>>
>>>> Shouldn't the WAKE_LEVEL be latched as well?
>>>>
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>>> {
>>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> @@ -2540,6 +2643,10 @@ static const struct pinctrl_pin_desc tegra210_pin_descs[] = {
>>>>> TEGRA210_IO_PAD_TABLE(TEGRA_IO_PIN_DESC)
>>>>> };
>>>>>
>>>>> +static const struct tegra_wake_event tegra210_wake_events[] = {
>>>>> + TEGRA_WAKE_IRQ("rtc", 16, 2),
>>>>> +};
>>>>> +
>>>>> static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>>> .num_powergates = ARRAY_SIZE(tegra210_powergates),
>>>>> .powergates = tegra210_powergates,
>>>>> @@ -2557,10 +2664,14 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>>>>> .regs = &tegra20_pmc_regs,
>>>>> .init = tegra20_pmc_init,
>>>>> .setup_irq_polarity = tegra20_pmc_setup_irq_polarity,
>>>>> + .irq_set_wake = tegra210_pmc_irq_set_wake,
>>>>> + .irq_set_type = tegra210_pmc_irq_set_type,
>>>>> .reset_sources = tegra210_reset_sources,
>>>>> .num_reset_sources = ARRAY_SIZE(tegra210_reset_sources),
>>>>> .reset_levels = NULL,
>>>>> .num_reset_levels = 0,
>>>>> + .num_wake_events = ARRAY_SIZE(tegra210_wake_events),
>>>>> + .wake_events = tegra210_wake_events,
>>>>> };
>>>>>
>>>>> #define TEGRA186_IO_PAD_TABLE(_pad) \
>>>>>
>>>>
>>>
>>
>
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: aspeed: tiogapass: Add VR devices
From: Joel Stanley @ 2019-07-23 2:16 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Vijay Khemka, Rob Herring, Mark Rutland, devicetree, Linux ARM,
linux-aspeed, Linux Kernel Mailing List,
openbmc @ lists . ozlabs . org, Sai Dasari
In-Reply-To: <a0a8162e-c21b-4b3d-b096-1676c5cc9758@www.fastmail.com>
On Tue, 23 Jul 2019 at 00:40, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Tue, 23 Jul 2019, at 10:04, Vijay Khemka wrote:
> > Adds voltage regulators Infineon pxe1610 devices to Facebook
> > tiogapass platform.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
>
> Acked-by: Andrew Jeffery <andrew@aj.id.au>
Thanks, applied to aspeed's dt-for-5.4.
Cheers,
Joel
^ permalink raw reply
* Re: [PATCH] dt-bindings: Add pxe1610 as a trivial device
From: Joel Stanley @ 2019-07-23 2:18 UTC (permalink / raw)
To: Vijay Khemka
Cc: Rob Herring, Mark Rutland, Jiri Kosina, Guenter Roeck, Herbert Xu,
Patrick Venture, Ard Biesheuvel, Anson Huang, Jeremy Gebben,
devicetree, Linux Kernel Mailing List,
openbmc @ lists . ozlabs . org, linux-aspeed, Sai Dasari
In-Reply-To: <20190723002052.2878847-1-vijaykhemka@fb.com>
On Tue, 23 Jul 2019 at 00:46, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> The pxe1610 is a voltage regulator from Infineon. It also supports
> other VRs pxe1110 and pxm1310 from Infineon.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Acked-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* Re: [PATCH v3 5/6] dt-bindings: interconnect: Add interconnect-opp-table property
From: Viresh Kumar @ 2019-07-23 2:21 UTC (permalink / raw)
To: Rob Herring
Cc: Saravana Kannan, Georgi Djakov, Mark Rutland, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, vincent.guittot,
seansw, daidavid1, Rajendra Nayak, sibis, bjorn.andersson,
evgreen, kernel-team, linux-pm, devicetree, linux-kernel
In-Reply-To: <20190722233927.GA1353@bogus>
On 22-07-19, 17:39, Rob Herring wrote:
> On Tue, Jul 02, 2019 at 06:10:19PM -0700, Saravana Kannan wrote:
> > Add support for listing bandwidth OPP tables for each interconnect path
> > listed using the interconnects property.
> >
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > .../devicetree/bindings/interconnect/interconnect.txt | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt
> > index 6f5d23a605b7..fc5b75b76a2c 100644
> > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt
> > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt
> > @@ -55,10 +55,18 @@ interconnect-names : List of interconnect path name strings sorted in the same
> > * dma-mem: Path from the device to the main memory of
> > the system
> >
> > +interconnect-opp-table: List of phandles to OPP tables (bandwidth OPP tables)
> > + that specify the OPPs for the interconnect paths listed
> > + in the interconnects property. This property can only
> > + point to OPP tables that belong to the device and are
> > + listed in the device's operating-points-v2 property.
> > +
>
> IMO, there's no need for this property. Which OPP is which should be
> defined already as part of the device's binding. That's enough for the
> driver to know which OPP applies to the interconnect.
And if there is confusion we can actually use the compatible property
to have another string which highlights that it is an interconnect OPP
?
--
viresh
^ permalink raw reply
* Re: [PATCH v4 2/3] dt-bindings: rng: add bindings for MediaTek ARMv8 SoCs
From: Neal Liu @ 2019-07-23 2:21 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree@vger.kernel.org, Herbert Xu,
wsd_upstream, Sean Wang, linux-kernel@vger.kernel.org,
Crystal Guo (郭晶), linux-crypto@vger.kernel.org,
Matt Mackall, Matthias Brugger,
linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190722171320.GA9806@bogus>
Dear Rob,
You can check my driver for detail:
http://patchwork.kernel.org/patch/11012475/ or patchset 3/3
This driver is registered as hardware random number generator, and
combines with rng-core.
We want to add one rng hw based on the dts. Is this proper or do you
have other suggestion to meet this requirement?
Thanks
On Tue, 2019-07-23 at 01:13 +0800, Rob Herring wrote:
> On Mon, Jun 24, 2019 at 03:24:11PM +0800, Neal Liu wrote:
> > Document the binding used by the MediaTek ARMv8 SoCs random
> > number generator with TrustZone enabled.
> >
> > Signed-off-by: Neal Liu <neal.liu@mediatek.com>
> > ---
> > .../devicetree/bindings/rng/mtk-sec-rng.txt | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> >
> > diff --git a/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt b/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> > new file mode 100644
> > index 0000000..c04ce15
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rng/mtk-sec-rng.txt
> > @@ -0,0 +1,10 @@
> > +MediaTek random number generator with TrustZone enabled
> > +
> > +Required properties:
> > +- compatible : Should be "mediatek,mtk-sec-rng"
>
> What's the interface to access this?
>
> A node with a 'compatible' and nothing else is a sign of something that
> a parent device should instantiate and doesn't need to be in DT. IOW,
> what do complete bindings for firmware functions look like?
>
> > +
> > +Example:
> > +
> > +hwrng: hwrng {
> > + compatible = "mediatek,mtk-sec-rng";
> > +}
> > --
> > 1.7.9.5
> >
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* RE: [PATCH V5 3/5] arm64: dts: imx8mm: Add system counter node
From: Anson Huang @ 2019-07-23 2:29 UTC (permalink / raw)
To: Shawn Guo, daniel.lezcano@linaro.org
Cc: mark.rutland@arm.com, Jacky Bai, ccaione@baylibre.com,
catalin.marinas@arm.com, agx@sigxcpu.org, angus@akkea.ca,
Leonard Crestez, festevam@gmail.com, Abel Vesa,
andrew.smirnov@gmail.com, will@kernel.org, dl-linux-imx,
devicetree@vger.kernel.org, s.hauer@pengutronix.de,
robh+dt@kernel.org, tglx@linutronix.de, Daniel Baluta,
linux-arm-kernel@lists.infradead.org, Aisheng Dong
In-Reply-To: <20190722031517.GT3738@dragon>
Hi, Shawn
> On Wed, Jul 10, 2019 at 02:30:54PM +0800, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > Add i.MX8MM system counter node to enable timer-imx-sysctr broadcast
> > timer driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>
> Do I need to wait for patch #1 landing before I apply #3 ~ #5, or can they be
> applied independently (no breaking on anything)?
Without #1, system can bootup, but the system counter's freq will be incorrect,
although it does NOT impact normal function. So I think it is better to wait for
#1 landing. @daniel.lezcano@linaro.org, can you help review the #1 patch, since
I use a different way to fix the clock issue which is more simple.
Anson
^ permalink raw reply
* Re: [PATCH v2 00/12] Clean up "mediatek,larb" after adding device_link
From: CK Hu @ 2019-07-23 2:31 UTC (permalink / raw)
To: Yong Wu
Cc: Matthias Brugger, Joerg Roedel, Rob Herring, youlin.pei,
devicetree, Nicolas Boichat, srv_heupstream, Will Deacon,
linux-kernel, Evan Green, Tomasz Figa, iommu, linux-mediatek,
yingjoe.chen, anan.sun, Robin Murphy, linux-arm-kernel
In-Reply-To: <1560171313-28299-1-git-send-email-yong.wu@mediatek.com>
Hi, Yong:
I've added log in mtk_smi_clk_enable() and mtk_smi_clk_disable(), and I
boot MT8183 with display, the log is
[ 4.020340] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[ 4.331371] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
[ 4.429578] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[ 4.719743] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
[ 5.084770] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[ 5.904310] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
>From the log, the clock is finally turned off, but the display works
normally. This is because scpsys has turn the clock on,
scpsys: syscon@10006000 {
compatible = "mediatek,mt8183-scpsys", "syscon";
#power-domain-cells = <1>;
reg = <0 0x10006000 0 0x1000>;
clocks = <&topckgen CLK_TOP_MUX_AUD_INTBUS>,
<&mmsys CLK_MM_SMI_COMMON>,
<&mmsys CLK_MM_GALS_COMM0>,
<&mmsys CLK_MM_GALS_COMM1>,
clock-names = "audio","mm-0",
"mm-1", "mm-2";
}
I'm worried that for MT8173, scpsys would not turn on subsys clock, this
series would let display work abnormally, so I think smi common should
not depend on scpsys to turn on the clock.
You could simply remove the clock parameter in scpsys device node, and
you would see the display works abnormally.
Regards,
CK
On Mon, 2019-06-10 at 20:55 +0800, Yong Wu wrote:
> MediaTek IOMMU block diagram always like below:
>
> M4U
> |
> smi-common
> |
> -------------
> | | ...
> | |
> larb1 larb2
> | |
> vdec venc
>
> All the consumer connect with smi-larb, then connect with smi-common.
>
> MediaTek IOMMU don't have its power-domain. When the consumer works,
> it should enable the smi-larb's power which also need enable the smi-common's
> power firstly.
>
> Thus, Firstly, use the device link connect the consumer and the
> smi-larbs. then add device link between the smi-larb and smi-common.
>
> After adding the device_link, then "mediatek,larb" property can be removed.
> the iommu consumer don't need call the mtk_smi_larb_get/put to enable
> the power and clock of smi-larb and smi-common.
>
> This patchset depends on "MT8183 IOMMU SUPPORT"[1].
>
> [1] https://lists.linuxfoundation.org/pipermail/iommu/2019-June/036552.html
>
> Change notes:
> v2:
> 1) rebase on v5.2-rc1.
> 2) Move adding device_link between the consumer and smi-larb into
> iommu_add_device from Robin.
> 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from Evan.
> 4) Remove the shutdown callback in iommu.
>
> v1: https://lists.linuxfoundation.org/pipermail/iommu/2019-January/032387.html
>
> Yong Wu (12):
> dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW
> iommu/mediatek: Add probe_defer for smi-larb
> iommu/mediatek: Add device_link between the consumer and the larb
> devices
> memory: mtk-smi: Add device-link between smi-larb and smi-common
> media: mtk-jpeg: Get rid of mtk_smi_larb_get/put
> media: mtk-mdp: Get rid of mtk_smi_larb_get/put
> media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
> drm/mediatek: Get rid of mtk_smi_larb_get/put
> memory: mtk-smi: Get rid of mtk_smi_larb_get/put
> iommu/mediatek: Use builtin_platform_driver
> arm: dts: mediatek: Get rid of mediatek,larb for MM nodes
> arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes
>
> .../bindings/display/mediatek/mediatek,disp.txt | 9 -----
> .../bindings/media/mediatek-jpeg-decoder.txt | 4 --
> .../devicetree/bindings/media/mediatek-mdp.txt | 8 ----
> .../devicetree/bindings/media/mediatek-vcodec.txt | 4 --
> arch/arm/boot/dts/mt2701.dtsi | 1 -
> arch/arm/boot/dts/mt7623.dtsi | 1 -
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 -------
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 11 -----
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 26 ------------
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 -
> drivers/iommu/mtk_iommu.c | 45 +++++++--------------
> drivers/iommu/mtk_iommu_v1.c | 39 +++++++-----------
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 22 ----------
> drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 -
> drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 38 -----------------
> drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 -
> drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 -
> .../media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 21 ----------
> drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 3 --
> drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 1 -
> .../media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 47 ----------------------
> drivers/memory/mtk-smi.c | 31 ++++----------
> include/soc/mediatek/smi.h | 20 ---------
> 23 files changed, 36 insertions(+), 316 deletions(-)
>
^ permalink raw reply
* Re: [PATCH RESEND 1/1] dt-bindings: serial: lpuart: add the clock requirement for imx8qxp
From: Shawn Guo @ 2019-07-23 2:46 UTC (permalink / raw)
To: fugang.duan
Cc: robh+dt, mark.rutland, gregkh, festevam, daniel.baluta,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20190704134355.2402-1-fugang.duan@nxp.com>
On Thu, Jul 04, 2019 at 09:43:55PM +0800, fugang.duan@nxp.com wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
>
> Add the baud clock requirement for imx8qxp.
>
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
From: Shawn Guo @ 2019-07-23 2:50 UTC (permalink / raw)
To: Anson.Huang
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
s.hauer, kernel, festevam, mturquette, sboyd, l.stach, abel.vesa,
andrew.smirnov, angus, ccaione, agx, leonard.crestez, linux-pm,
devicetree, linux-kernel, linux-arm-kernel, linux-clk, Linux-imx
In-Reply-To: <20190705045612.27665-5-Anson.Huang@nxp.com>
On Fri, Jul 05, 2019 at 12:56:11PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
>
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 6/6] arm64: dts: imx8mq: Add clock for TMU node
From: Shawn Guo @ 2019-07-23 2:51 UTC (permalink / raw)
To: Anson.Huang
Cc: rui.zhang, edubezval, daniel.lezcano, robh+dt, mark.rutland,
s.hauer, kernel, festevam, mturquette, sboyd, l.stach, abel.vesa,
andrew.smirnov, angus, ccaione, agx, leonard.crestez, linux-pm,
devicetree, linux-kernel, linux-arm-kernel, linux-clk, Linux-imx
In-Reply-To: <20190705045612.27665-6-Anson.Huang@nxp.com>
On Fri, Jul 05, 2019 at 12:56:12PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
>
> i.MX8MQ has clock gate for TMU module, add clock info to TMU
> node for clock management.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH V4 2/2] arm64: dts: imx8mm: Add "fsl,imx8mq-src" as src's fallback compatible
From: Shawn Guo @ 2019-07-23 2:57 UTC (permalink / raw)
To: Anson.Huang
Cc: p.zabel, robh+dt, mark.rutland, s.hauer, kernel, festevam,
leonard.crestez, viresh.kumar, daniel.baluta, ping.bai,
devicetree, linux-arm-kernel, linux-kernel, Linux-imx
In-Reply-To: <20190705085406.22483-2-Anson.Huang@nxp.com>
On Fri, Jul 05, 2019 at 04:54:06PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
>
> i.MX8MM can reuse i.MX8MQ's src driver, add "fsl,imx8mq-src" as
> src's fallback compatible to enable it.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Applied this one, thanks.
^ permalink raw reply
* Re: [PATCH V4 1/2] dt-bindings: reset: imx7: Add support for i.MX8MM
From: Shawn Guo @ 2019-07-23 2:59 UTC (permalink / raw)
To: Anson.Huang, p.zabel
Cc: robh+dt, mark.rutland, s.hauer, kernel, festevam, leonard.crestez,
viresh.kumar, daniel.baluta, ping.bai, devicetree,
linux-arm-kernel, linux-kernel, Linux-imx
In-Reply-To: <20190705085406.22483-1-Anson.Huang@nxp.com>
On Fri, Jul 05, 2019 at 04:54:05PM +0800, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
>
> i.MX8MM can reuse i.MX8MQ's reset driver, update the compatible
> property and related info to support i.MX8MM.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Hi Philipp,
Let me know if you want me to pick this up.
Shawn
> ---
> Changes since V3:
> - Add comments to those reset indices to indicate which are NOT supported on i.MX8MM.
> ---
> .../devicetree/bindings/reset/fsl,imx7-src.txt | 6 +++--
> include/dt-bindings/reset/imx8mq-reset.h | 28 +++++++++++-----------
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> index 13e0951..c2489e4 100644
> --- a/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> +++ b/Documentation/devicetree/bindings/reset/fsl,imx7-src.txt
> @@ -8,6 +8,7 @@ Required properties:
> - compatible:
> - For i.MX7 SoCs should be "fsl,imx7d-src", "syscon"
> - For i.MX8MQ SoCs should be "fsl,imx8mq-src", "syscon"
> + - For i.MX8MM SoCs should be "fsl,imx8mm-src", "fsl,imx8mq-src", "syscon"
> - reg: should be register base and length as documented in the
> datasheet
> - interrupts: Should contain SRC interrupt
> @@ -46,5 +47,6 @@ Example:
>
>
> For list of all valid reset indices see
> -<dt-bindings/reset/imx7-reset.h> for i.MX7 and
> -<dt-bindings/reset/imx8mq-reset.h> for i.MX8MQ
> +<dt-bindings/reset/imx7-reset.h> for i.MX7,
> +<dt-bindings/reset/imx8mq-reset.h> for i.MX8MQ and
> +<dt-bindings/reset/imx8mq-reset.h> for i.MX8MM
> diff --git a/include/dt-bindings/reset/imx8mq-reset.h b/include/dt-bindings/reset/imx8mq-reset.h
> index 57c5924..f17ef2a 100644
> --- a/include/dt-bindings/reset/imx8mq-reset.h
> +++ b/include/dt-bindings/reset/imx8mq-reset.h
> @@ -38,26 +38,26 @@
> #define IMX8MQ_RESET_PCIEPHY_PERST 27
> #define IMX8MQ_RESET_PCIE_CTRL_APPS_EN 28
> #define IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF 29
> -#define IMX8MQ_RESET_HDMI_PHY_APB_RESET 30
> +#define IMX8MQ_RESET_HDMI_PHY_APB_RESET 30 /* i.MX8MM does NOT support */
> #define IMX8MQ_RESET_DISP_RESET 31
> #define IMX8MQ_RESET_GPU_RESET 32
> #define IMX8MQ_RESET_VPU_RESET 33
> -#define IMX8MQ_RESET_PCIEPHY2 34
> -#define IMX8MQ_RESET_PCIEPHY2_PERST 35
> -#define IMX8MQ_RESET_PCIE2_CTRL_APPS_EN 36
> -#define IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF 37
> -#define IMX8MQ_RESET_MIPI_CSI1_CORE_RESET 38
> -#define IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET 39
> -#define IMX8MQ_RESET_MIPI_CSI1_ESC_RESET 40
> -#define IMX8MQ_RESET_MIPI_CSI2_CORE_RESET 41
> -#define IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET 42
> -#define IMX8MQ_RESET_MIPI_CSI2_ESC_RESET 43
> +#define IMX8MQ_RESET_PCIEPHY2 34 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_PCIEPHY2_PERST 35 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_PCIE2_CTRL_APPS_EN 36 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_PCIE2_CTRL_APPS_TURNOFF 37 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI1_CORE_RESET 38 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI1_PHY_REF_RESET 39 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI1_ESC_RESET 40 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI2_CORE_RESET 41 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI2_PHY_REF_RESET 42 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_MIPI_CSI2_ESC_RESET 43 /* i.MX8MM does NOT support */
> #define IMX8MQ_RESET_DDRC1_PRST 44
> #define IMX8MQ_RESET_DDRC1_CORE_RESET 45
> #define IMX8MQ_RESET_DDRC1_PHY_RESET 46
> -#define IMX8MQ_RESET_DDRC2_PRST 47
> -#define IMX8MQ_RESET_DDRC2_CORE_RESET 48
> -#define IMX8MQ_RESET_DDRC2_PHY_RESET 49
> +#define IMX8MQ_RESET_DDRC2_PRST 47 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_DDRC2_CORE_RESET 48 /* i.MX8MM does NOT support */
> +#define IMX8MQ_RESET_DDRC2_PHY_RESET 49 /* i.MX8MM does NOT support */
>
> #define IMX8MQ_RESET_NUM 50
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23 3:03 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: <71a88a9c-a542-557a-0eaa-3c90112dee0e@nvidia.com>
23.07.2019 4:52, Sowjanya Komatineni пишет:
>
> On 7/22/19 6:41 PM, Dmitry Osipenko wrote:
>> 23.07.2019 4:08, Dmitry Osipenko пишет:
>>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>>> common used RTC alarm wake event.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 111 insertions(+)
>>>>>
>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>> @@ -57,6 +57,12 @@
>>>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>>> Please follow the TRM's bits naming.
>>>
>>> PMC_CNTRL_LATCHWAKE_EN
>>>
>>>>> +#define PMC_WAKE_MASK 0x0c
>>>>> +#define PMC_WAKE_LEVEL 0x10
>>>>> +#define PMC_WAKE_STATUS 0x14
>>>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>>>
>>>>> #define DPD_SAMPLE 0x020
>>>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>>>> @@ -87,6 +93,11 @@
>>>>>
>>>>> #define PMC_SCRATCH41 0x140
>>>>>
>>>>> +#define PMC_WAKE2_MASK 0x160
>>>>> +#define PMC_WAKE2_LEVEL 0x164
>>>>> +#define PMC_WAKE2_STATUS 0x168
>>>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>>>> +
>>>>> #define PMC_SENSOR_CTRL 0x1b0
>>>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>>>> .alloc = tegra_pmc_irq_alloc,
>>>>> };
>>>>>
>>>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> +{
>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> + unsigned int offset, bit;
>>>>> + u32 value;
>>>>> +
>>>>> + if (data->hwirq == ULONG_MAX)
>>>>> + return 0;
>>>>> +
>>>>> + offset = data->hwirq / 32;
>>>>> + bit = data->hwirq % 32;
>>>>> +
>>>>> + /*
>>>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>>> + * that would not make it into wakeup event register during LP0 exit.
>>>>> + */
>>>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>> + udelay(120);
>>>> Why it takes so much time to latch the values? Shouldn't some status-bit
>>>> be polled for the completion of latching?
>>>>
>>>> Is this register-write really getting buffered in the PMC?
>>>>
>>>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>> + udelay(120);
>>>> 120 usecs to remove latching, really?
>>>>
>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>>> +
>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>>> +
>>>>> + /* enable PMC wake */
>>>>> + if (data->hwirq >= 32)
>>>>> + offset = PMC_WAKE2_MASK;
>>>>> + else
>>>>> + offset = PMC_WAKE_MASK;
>>>>> +
>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>> +
>>>>> + if (on)
>>>>> + value |= 1 << bit;
>>>>> + else
>>>>> + value &= ~(1 << bit);
>>>>> +
>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>> Why the latching is done *before* writing into the WAKE registers? What
>>>> it is latching then?
>>> I'm looking at the TRM doc and it says that latching should be done
>>> *after* writing to the WAKE_MASK / LEVEL registers.
>>>
>>> Secondly it says that it's enough to do:
>>>
>>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>
>>> in order to latch. There is no need for the delay and to remove the
>>> "LATCHWAKE_EN" bit, it should be a oneshot action.
>> Although, no. TRM says "stops latching on transition from 1
>> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>>
>> Have you tested this code at all? I'm wondering how it happens to work
>> without a proper latching.
> Yes, ofcourse its tested and this sequence to do transition is
> recommendation from Tegra designer.
> Will check if TRM doesn't have update properly or will re-confirm
> internally on delay time...
>
> On any of the wake event PMC wakeup happens and WAKE_STATUS register
> will have bits set for all events that triggered wake.
> After wakeup PMC doesn't update SW_WAKE_STATUS register as per PMC design.
> SW latch register added in design helps to provide a way to capture
> those events that happen right during wakeup time and didnt make it to
> SW_WAKE_STATUS register.
> So before next suspend entry, latching all prior wake events into SW
> WAKE_STATUS and then clearing them.
I'm now wondering whether the latching cold be turned ON permanently
during of the PMC's probe, for simplicity.
> LATCHWAKE_EN - When set, enables latching and stops latching on
> transition from 1 to 0
> There is recommendation of min 120uSec for this transition to stop
> latching. Will double-check why 120uSec
Yes, please check.
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> {
>>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>>> +{
>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>> + unsigned int offset, bit;
>>>>> + u32 value;
>>>>> +
>>>>> + if (data->hwirq == ULONG_MAX)
>>>>> + return 0;
>>>>> +
>>>>> + offset = data->hwirq / 32;
>>>>> + bit = data->hwirq % 32;
>>>>> +
>>>>> + if (data->hwirq >= 32)
>>>>> + offset = PMC_WAKE2_LEVEL;
>>>>> + else
>>>>> + offset = PMC_WAKE_LEVEL;
>>>>> +
>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>> +
>>>>> + switch (type) {
>>>>> + case IRQ_TYPE_EDGE_RISING:
>>>>> + case IRQ_TYPE_LEVEL_HIGH:
>>>>> + value |= 1 << bit;
>>>>> + break;
>>>>> +
>>>>> + case IRQ_TYPE_EDGE_FALLING:
>>>>> + case IRQ_TYPE_LEVEL_LOW:
>>>>> + value &= ~(1 << bit);
>>>>> + break;
>>>>> +
>>>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>>>> + value ^= 1 << bit;
>>>>> + break;
>>>>> +
>>>>> + default:
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>> Shouldn't the WAKE_LEVEL be latched as well?
> WAKE_LEVELs dont need any latch as they are the levels SW sets for wake
> trigger and they are not status
Okay.
[snip]
^ permalink raw reply
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Sowjanya Komatineni @ 2019-07-23 3:09 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: <70ad28cb-c268-cbbe-36f5-39df26617d8e@gmail.com>
On 7/22/19 8:03 PM, Dmitry Osipenko wrote:
> 23.07.2019 4:52, Sowjanya Komatineni пишет:
>> On 7/22/19 6:41 PM, Dmitry Osipenko wrote:
>>> 23.07.2019 4:08, Dmitry Osipenko пишет:
>>>> 23.07.2019 3:58, Dmitry Osipenko пишет:
>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>>>>> This patch implements PMC wakeup sequence for Tegra210 and defines
>>>>>> common used RTC alarm wake event.
>>>>>>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>> drivers/soc/tegra/pmc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 111 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>>> index 91c84d0e66ae..c556f38874e1 100644
>>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>>> @@ -57,6 +57,12 @@
>>>>>> #define PMC_CNTRL_SYSCLK_OE BIT(11) /* system clock enable */
>>>>>> #define PMC_CNTRL_SYSCLK_POLARITY BIT(10) /* sys clk polarity */
>>>>>> #define PMC_CNTRL_MAIN_RST BIT(4)
>>>>>> +#define PMC_CNTRL_LATCH_WAKEUPS BIT(5)
>>>> Please follow the TRM's bits naming.
>>>>
>>>> PMC_CNTRL_LATCHWAKE_EN
>>>>
>>>>>> +#define PMC_WAKE_MASK 0x0c
>>>>>> +#define PMC_WAKE_LEVEL 0x10
>>>>>> +#define PMC_WAKE_STATUS 0x14
>>>>>> +#define PMC_SW_WAKE_STATUS 0x18
>>>>>>
>>>>>> #define DPD_SAMPLE 0x020
>>>>>> #define DPD_SAMPLE_ENABLE BIT(0)
>>>>>> @@ -87,6 +93,11 @@
>>>>>>
>>>>>> #define PMC_SCRATCH41 0x140
>>>>>>
>>>>>> +#define PMC_WAKE2_MASK 0x160
>>>>>> +#define PMC_WAKE2_LEVEL 0x164
>>>>>> +#define PMC_WAKE2_STATUS 0x168
>>>>>> +#define PMC_SW_WAKE2_STATUS 0x16c
>>>>>> +
>>>>>> #define PMC_SENSOR_CTRL 0x1b0
>>>>>> #define PMC_SENSOR_CTRL_SCRATCH_WRITE BIT(2)
>>>>>> #define PMC_SENSOR_CTRL_ENABLE_RST BIT(1)
>>>>>> @@ -1922,6 +1933,55 @@ static const struct irq_domain_ops tegra_pmc_irq_domain_ops = {
>>>>>> .alloc = tegra_pmc_irq_alloc,
>>>>>> };
>>>>>>
>>>>>> +static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>>> +{
>>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>>> + unsigned int offset, bit;
>>>>>> + u32 value;
>>>>>> +
>>>>>> + if (data->hwirq == ULONG_MAX)
>>>>>> + return 0;
>>>>>> +
>>>>>> + offset = data->hwirq / 32;
>>>>>> + bit = data->hwirq % 32;
>>>>>> +
>>>>>> + /*
>>>>>> + * Latch wakeups to SW_WAKE_STATUS register to capture events
>>>>>> + * that would not make it into wakeup event register during LP0 exit.
>>>>>> + */
>>>>>> + value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>>>> + value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>>> + udelay(120);
>>>>> Why it takes so much time to latch the values? Shouldn't some status-bit
>>>>> be polled for the completion of latching?
>>>>>
>>>>> Is this register-write really getting buffered in the PMC?
>>>>>
>>>>>> + value &= ~PMC_CNTRL_LATCH_WAKEUPS;
>>>>>> + tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>>> + udelay(120);
>>>>> 120 usecs to remove latching, really?
>>>>>
>>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE_STATUS);
>>>>>> + tegra_pmc_writel(pmc, 0, PMC_SW_WAKE2_STATUS);
>>>>>> +
>>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE_STATUS);
>>>>>> + tegra_pmc_writel(pmc, 0, PMC_WAKE2_STATUS);
>>>>>> +
>>>>>> + /* enable PMC wake */
>>>>>> + if (data->hwirq >= 32)
>>>>>> + offset = PMC_WAKE2_MASK;
>>>>>> + else
>>>>>> + offset = PMC_WAKE_MASK;
>>>>>> +
>>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>>> +
>>>>>> + if (on)
>>>>>> + value |= 1 << bit;
>>>>>> + else
>>>>>> + value &= ~(1 << bit);
>>>>>> +
>>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>>> Why the latching is done *before* writing into the WAKE registers? What
>>>>> it is latching then?
>>>> I'm looking at the TRM doc and it says that latching should be done
>>>> *after* writing to the WAKE_MASK / LEVEL registers.
>>>>
>>>> Secondly it says that it's enough to do:
>>>>
>>>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>>>> value |= PMC_CNTRL_LATCH_WAKEUPS;
>>>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>>>>
>>>> in order to latch. There is no need for the delay and to remove the
>>>> "LATCHWAKE_EN" bit, it should be a oneshot action.
>>> Although, no. TRM says "stops latching on transition from 1
>>> to 0 (sequence - set to 1,set to 0)", so it's not a oneshot action.
>>>
>>> Have you tested this code at all? I'm wondering how it happens to work
>>> without a proper latching.
>> Yes, ofcourse its tested and this sequence to do transition is
>> recommendation from Tegra designer.
>> Will check if TRM doesn't have update properly or will re-confirm
>> internally on delay time...
>>
>> On any of the wake event PMC wakeup happens and WAKE_STATUS register
>> will have bits set for all events that triggered wake.
>> After wakeup PMC doesn't update SW_WAKE_STATUS register as per PMC design.
>> SW latch register added in design helps to provide a way to capture
>> those events that happen right during wakeup time and didnt make it to
>> SW_WAKE_STATUS register.
>> So before next suspend entry, latching all prior wake events into SW
>> WAKE_STATUS and then clearing them.
> I'm now wondering whether the latching cold be turned ON permanently
> during of the PMC's probe, for simplicity.
latching should be done on suspend-resume cycle as wake events gets
generates on every suspend-resume cycle.
>> LATCHWAKE_EN - When set, enables latching and stops latching on
>> transition from 1 to 0
>> There is recommendation of min 120uSec for this transition to stop
>> latching. Will double-check why 120uSec
> Yes, please check.
>
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>>> {
>>>>>> struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>>> @@ -1954,6 +2014,49 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
>>>>>> +{
>>>>>> + struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
>>>>>> + unsigned int offset, bit;
>>>>>> + u32 value;
>>>>>> +
>>>>>> + if (data->hwirq == ULONG_MAX)
>>>>>> + return 0;
>>>>>> +
>>>>>> + offset = data->hwirq / 32;
>>>>>> + bit = data->hwirq % 32;
>>>>>> +
>>>>>> + if (data->hwirq >= 32)
>>>>>> + offset = PMC_WAKE2_LEVEL;
>>>>>> + else
>>>>>> + offset = PMC_WAKE_LEVEL;
>>>>>> +
>>>>>> + value = tegra_pmc_readl(pmc, offset);
>>>>>> +
>>>>>> + switch (type) {
>>>>>> + case IRQ_TYPE_EDGE_RISING:
>>>>>> + case IRQ_TYPE_LEVEL_HIGH:
>>>>>> + value |= 1 << bit;
>>>>>> + break;
>>>>>> +
>>>>>> + case IRQ_TYPE_EDGE_FALLING:
>>>>>> + case IRQ_TYPE_LEVEL_LOW:
>>>>>> + value &= ~(1 << bit);
>>>>>> + break;
>>>>>> +
>>>>>> + case IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING:
>>>>>> + value ^= 1 << bit;
>>>>>> + break;
>>>>>> +
>>>>>> + default:
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + tegra_pmc_writel(pmc, value, offset);
>>>>> Shouldn't the WAKE_LEVEL be latched as well?
>> WAKE_LEVELs dont need any latch as they are the levels SW sets for wake
>> trigger and they are not status
> Okay.
>
> [snip]
^ permalink raw reply
* Re: [PATCH 02/10] ARM: dts: imx6ul: segin: Add boot media to dts filename
From: Shawn Guo @ 2019-07-23 3:10 UTC (permalink / raw)
To: Stefan Riedmueller
Cc: s.hauer, robh+dt, mark.rutland, devicetree, martyn.welch,
linux-kernel, linux-imx, kernel, festevam, linux-arm-kernel
In-Reply-To: <1562656767-273566-3-git-send-email-s.riedmueller@phytec.de>
On Tue, Jul 09, 2019 at 09:19:19AM +0200, Stefan Riedmueller wrote:
> There is now a PHYTEC phyCORE-i.MX 6UL with eMMC instead of NAND flash
> available. The dts filename needs to reflect that to differentiate both.
>
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
> arch/arm/boot/dts/Makefile | 2 +-
> ...l-phytec-segin-ff-rdk.dts => imx6ul-phytec-segin-ff-rdk-nand.dts} | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
> rename arch/arm/boot/dts/{imx6ul-phytec-segin-ff-rdk.dts => imx6ul-phytec-segin-ff-rdk-nand.dts} (85%)
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e1924b06f3cb..668b57c8cc57 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -573,7 +573,7 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
> imx6ul-opos6uldev.dtb \
> imx6ul-pico-hobbit.dtb \
> imx6ul-pico-pi.dtb \
> - imx6ul-phytec-segin-ff-rdk.dtb \
> + imx6ul-phytec-segin-ff-rdk-nand.dtb \
> imx6ul-tx6ul-0010.dtb \
> imx6ul-tx6ul-0011.dtb \
> imx6ul-tx6ul-mainboard.dtb \
> diff --git a/arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk.dts b/arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk-nand.dts
> similarity index 85%
> rename from arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk.dts
> rename to arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk-nand.dts
> index 1e59183a2f7c..dc06029c5701 100644
> --- a/arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk.dts
> +++ b/arch/arm/boot/dts/imx6ul-phytec-segin-ff-rdk-nand.dts
> @@ -10,8 +10,9 @@
> #include "imx6ul-phytec-segin-peb-eval-01.dtsi"
>
> / {
> - model = "PHYTEC phyBOARD-Segin i.MX6 UltraLite Full Featured";
> - compatible = "phytec,imx6ul-pbacd10", "phytec,imx6ul-pcl063", "fsl,imx6ul";
> + model = "PHYTEC phyBOARD-Segin i.MX6 UltraLite Full Featured with NAND";
> + compatible = "phytec,imx6ul-pbacd10-nand", "phytec,imx6ul-pbacd10",
The board compatibles need to be documented.
Shawn
> + "phytec,imx6ul-pcl063", "fsl,imx6ul";
> };
>
> &adc1 {
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v8 00/15] memory: tegra: Introduce Tegra30 EMC driver
From: Dmitry Osipenko @ 2019-07-23 3:12 UTC (permalink / raw)
To: Rob Herring, Michael Turquette, Joseph Lo, Thierry Reding,
Jonathan Hunter, Peter De Schrijver, Prashant Gaikwad,
Stephen Boyd
Cc: devicetree, linux-clk, linux-tegra, linux-kernel
Hello,
This series introduces driver for the External Memory Controller (EMC)
found on Tegra30 chips, it controls the external DRAM on the board. The
purpose of this driver is to program memory timing for external memory on
the EMC clock rate change. The driver was tested using the ACTMON devfreq
driver that performs memory frequency scaling based on memory-usage load.
Changelog:
v8: - Added two new patches:
memory: tegra20-emc: Increase handshake timeout
memory: tegra20-emc: wait_for_completion_timeout() doesn't return error
Turned out that memory-clk handshake may take much more time under
some circumstances. The second patch is a minor cleanup. The same
changes are also applied to the Terga30 EMC driver addition-patch.
The pattern-properties of YAML bindings gained "type: object", for
consistency.
v7: - Addressed review comments that were made by Rob Herring to v6 by
removing old Terga30 Memory Controller binding once it's converted
to YAML, by using explicit patterns for the sub-nodes and specifying
min/max clock rates in the YAML.
- Two patches that were added in v6 are removed from the series:
clk: tegra20: emc: Add tegra20_clk_emc_on_pllp()
ARM: tegra30: cpuidle: Don't enter LP2 on CPU0 when EMC runs off PLLP
Because the problem with the PLLP is resolved now, turned out it was
a bug in the CPU-suspend code.
- The "Introduce Tegra30 EMC driver" patch got a fix for the "Same Freq"
bit typo, it's a bit 27 and not 16.
v6: - Tegra124 Memory Controller binding factored out into standalone
binding because it requires to specify MC_EMEM_ARB_MISC1 for EMEM
programming, which is not required for Tegra30. This makes the
upstream MC registers specification to match downstream exactly,
easing porting of boards memory timings configuration to upstream.
- Tegra30/124 Memory Controller binding converted to YAML.
- Tegra30 External Memory Controller binding now is in YAML format.
- Added workaround for hanging during LP2 when EMC runs off PLLP on
Tegra30 in this new patches:
clk: tegra20: emc: Add tegra20_clk_emc_on_pllp()
ARM: tegra30: cpuidle: Don't enter LP2 on CPU0 when EMC runs off PLLP
- Added info message to the Tegra20/30 EMC drivers, telling about
RAM code and a number of available timings:
memory: tegra20-emc: Print a brief info message about the timings
v5: - Addressed review comments that were made by Thierry Reding to v4 by
adding appropriate copyrights to the source code headers and making
Tegra30 EMC driver to use common Tegra20 CLK API directly instead
of having a dummy-proxy functions specifically for Tegra30.
- Addressed review comments that were made by Stephen Boyd to v4 by
rewording commit message of the "Add custom EMC clock implementation"
patch and adding clarifying comment (to that patch as well) which
tells why EMC is a critical clock.
- Added suspend-resume to Tegra30 EMC driver to error out if EMC driver
is in a "bad state" as it will likely cause a hang on entering suspend.
- Dropped patch "tegra20-emc: Replace clk_get_sys with devm_clk_get"
because the replaced clocks are actually should be removed altogether
in the "Drop setting EMC rate to max on probe" patch and that was
missed by an accident.
- Added "tegra20-emc: Pre-configure debug register" patch which ensures
that inappropriate HW debug features are disabled at a probe time.
The same change is also made in the "Introduce Tegra30 EMC driver"
patch.
- Added ACKs to the patches from Peter De Schrijver that he gave to v4
since all of the v5 changes are actually very minor.
v4: - Addressed review comments that were made by Peter De Schrijver to v3
by adding fence_udelay() after writes in the "Add custom EMC clock
implementation" patch.
- Added two new minor patches:
memory: tegra: Ensure timing control debug features are disabled
memory: tegra: Consolidate registers definition into one place
The first one is needed to ensure that EMC driver will work
properly regardless of hardware configuration left after boot.
The second patch is just a minor code cleanup.
- The "Introduce Tegra30 EMC driver" got also few very minor changes.
Now every possible error case is handled, nothing is ignored.
The EMC_DBG register is explicitly initialized during probe to be
on the safe side.
v3: - Addressed review comments that were made by Stephen Boyd to v2 by
adding explicit typing for the callback variable, by including
"clk-provider.h" directly in the code and by dropping __clk_lookup
usage where possible.
- Added more patches into this series:
memory: tegra20-emc: Drop setting EMC rate to max on probe
memory: tegra20-emc: Adapt for clock driver changes
memory: tegra20-emc: Include io.h instead of iopoll.h
memory: tegra20-emc: Replace clk_get_sys with devm_clk_get
Initially I was going to include these patches into other patchset,
but changed my mind after rearranging things a tad. The "Adapt for
clock driver changes" patch is directly related to the clock changes
done in the first patch of this series, the rest are minor cleanups
that are fine to include here as well.
- Added some more words to the commit message of "Add binding for NVIDIA
Tegra30 External Memory Controller" patch, clarifying why common DDR
timing device-tree form isn't suitable for Tegra30.
- The Tegra30 EMC driver now explicitly selects the registers access
mode (EMC_DBG mux), not relying on the setting left from bootloader.
v2: - Added support for changing MC clock diver configuration based on
Memory Controller (MC) configuration which is part of the memory
timing.
- Merged the "Add custom EMC clock implementation" patch into this
series because the "Introduce Tegra30 EMC driver" patch directly
depends on it. Please note that Tegra20 EMC driver will need to be
adapted for the clock changes as well, I'll send out the Tegra20
patches after this series will be applied because of some other
dependencies (devfreq) and because the temporary breakage won't
be critical (driver will just error out on probe).
- EMC driver now performs MC configuration validation by checking
that the number of MC / EMC timings matches and that the timings
rate is the same.
- EMC driver now supports timings that want to change the MC clock
configuration.
- Other minor prettifying changes of the code.
Dmitry Osipenko (15):
clk: tegra20/30: Add custom EMC clock implementation
memory: tegra20-emc: Drop setting EMC rate to max on probe
memory: tegra20-emc: Adapt for clock driver changes
memory: tegra20-emc: Include io.h instead of iopoll.h
memory: tegra20-emc: Pre-configure debug register
memory: tegra20-emc: Print a brief info message about the timings
memory: tegra20-emc: Increase handshake timeout
memory: tegra20-emc: wait_for_completion_timeout() doesn't return
error
dt-bindings: memory: tegra30: Convert to Tegra124 YAML
dt-bindings: memory: Add binding for NVIDIA Tegra30 Memory Controller
dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory
Controller
memory: tegra: Introduce Tegra30 EMC driver
memory: tegra: Ensure timing control debug features are disabled
memory: tegra: Consolidate registers definition into common header
ARM: dts: tegra30: Add External Memory Controller node
.../nvidia,tegra124-mc.yaml | 156 +++
.../nvidia,tegra30-emc.yaml | 341 +++++
.../memory-controllers/nvidia,tegra30-mc.txt | 123 --
.../memory-controllers/nvidia,tegra30-mc.yaml | 173 +++
arch/arm/boot/dts/tegra30.dtsi | 9 +
drivers/clk/tegra/Makefile | 2 +
drivers/clk/tegra/clk-tegra20-emc.c | 293 ++++
drivers/clk/tegra/clk-tegra20.c | 55 +-
drivers/clk/tegra/clk-tegra30.c | 38 +-
drivers/clk/tegra/clk.h | 3 +
drivers/memory/tegra/Kconfig | 10 +
drivers/memory/tegra/Makefile | 1 +
drivers/memory/tegra/mc.c | 42 +-
drivers/memory/tegra/mc.h | 74 +-
drivers/memory/tegra/tegra124.c | 20 -
drivers/memory/tegra/tegra20-emc.c | 134 +-
drivers/memory/tegra/tegra30-emc.c | 1230 +++++++++++++++++
drivers/memory/tegra/tegra30.c | 23 +
include/linux/clk/tegra.h | 11 +
include/soc/tegra/mc.h | 2 +-
20 files changed, 2427 insertions(+), 313 deletions(-)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra124-mc.yaml
create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
delete mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.txt
create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
create mode 100644 drivers/memory/tegra/tegra30-emc.c
--
2.22.0
^ 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