Linux GPIO subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH V6 16/21] soc/tegra: pmc: Add pmc wake support for tegra210
From: Dmitry Osipenko @ 2019-07-23  3:43 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: <97096b6c-f2f5-b82a-b172-802f4a06d1af@nvidia.com>

23.07.2019 6:31, Sowjanya Komatineni пишет:
> 
> On 7/22/19 8:25 PM, Dmitry Osipenko wrote:
>> 23.07.2019 6:09, Sowjanya Komatineni пишет:
>>> 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.
>> You're saying that PMC "doesn't update SW_WAKE_STATUS" after wake-up,
>> then I don't quite understand what's the point of disabling the latching
>> at all.
> When latch wake enable is set, events are latched and during 1 to 0
> transition latching is disabled.
> 
> This is to avoid sw_wake_status and wake_status showing diff events.

Okay.

> Currently driver is not relying on SW_WAKE_STATUS but its good to latch
> and clear so even at some point for some reason when SW_WAKE_STATUS is
> used, this wlil not cause mismatch with wake_status.

Then the latching need to be enabled on suspend and disabled early on
resume to get a proper WAKE status.

[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: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: <71c8cab1-bf72-c073-be30-4263c6b7c871@gmail.com>


On 7/22/19 8:25 PM, Dmitry Osipenko wrote:
> 23.07.2019 6:09, Sowjanya Komatineni пишет:
>> 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.
> You're saying that PMC "doesn't update SW_WAKE_STATUS" after wake-up,
> then I don't quite understand what's the point of disabling the latching
> at all.
When latch wake enable is set, events are latched and during 1 to 0 
transition latching is disabled.

This is to avoid sw_wake_status and wake_status showing diff events.

Currently driver is not relying on SW_WAKE_STATUS but its good to latch 
and clear so even at some point for some reason when SW_WAKE_STATUS is 
used, this wlil not cause mismatch with wake_status.

>>>> 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: Dmitry Osipenko @ 2019-07-23  3:25 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: <629826f9-c453-386a-9e88-bd64d23b8eab@nvidia.com>

23.07.2019 6:09, Sowjanya Komatineni пишет:
> 
> 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.

You're saying that PMC "doesn't update SW_WAKE_STATUS" after wake-up,
then I don't quite understand what's the point of disabling the latching
at all.

>>> 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 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: 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 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 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 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  0:58 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: <1563738060-30213-17-git-send-email-skomatineni@nvidia.com>

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)
> +
> +#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?

> +	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] pinctrl: msm8998: Squash TSIF pins together
From: Rob Herring @ 2019-07-22 23:53 UTC (permalink / raw)
  To: Marc Gonzalez
  Cc: Bjorn Andersson, Jonathan Neuschäfer, Jeffrey Hugo,
	Linus Walleij, MSM, Linux ARM, gpio, DT
In-Reply-To: <503b2ae8-ead6-70cd-7b21-ce5f5166a23a@free.fr>

On Thu, Jul 04, 2019 at 10:57:06AM +0200, Marc Gonzalez wrote:
> TSIF is the Transport Stream Interface.
> First, rename tsif1 to tsif0, and tsif2 to tsif1.
> Then squash all 5 tsif0 pins into a single function.
> Same for tsif1.

Doesn't this break backwards compatibility? If so, you should say so and 
say why that's okay for this platform. In any case, whether it's a 
problem or not is up to the platform maintainer(s). 

> 
> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
> ---
> Changes from v1:
> - Reword commit message for clarity (hopefully)
> - Drop unrelated change in qcom,msm8998-pinctrl.txt
> - CC DT
> ---
>  .../bindings/pinctrl/qcom,msm8998-pinctrl.txt |  5 +-

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/pinctrl/qcom/pinctrl-msm8998.c        | 76 +++++--------------
>  2 files changed, 20 insertions(+), 61 deletions(-)

^ permalink raw reply

* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Dmitry Osipenko @ 2019-07-22 23:35 UTC (permalink / raw)
  To: Marc Zyngier, Sowjanya Komatineni
  Cc: thierry.reding, jonathanh, tglx, jason, linus.walleij, stefan,
	mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
	linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
	mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <20190722193838.0d7cd2ad@why>

22.07.2019 21:38, Marc Zyngier пишет:
> On Mon, 22 Jul 2019 09:21:21 -0700
> Sowjanya Komatineni <skomatineni@nvidia.com> wrote:
> 
>> On 7/22/19 3:57 AM, Dmitry Osipenko wrote:
>>> 22.07.2019 13:13, Marc Zyngier пишет:  
>>>> On 22/07/2019 10:54, Dmitry Osipenko wrote:  
>>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:  
>>>>>> Tegra210 platforms use sc7 entry firmware to program Tegra LP0/SC7 entry
>>>>>> sequence and sc7 entry firmware is run from COP/BPMP-Lite.
>>>>>>
>>>>>> So, COP/BPMP-Lite still need IRQ function to finish SC7 suspend sequence
>>>>>> for Tegra210.
>>>>>>
>>>>>> This patch has fix for leaving the COP IRQ enabled for Tegra210 during
>>>>>> interrupt controller suspend operation.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>>   drivers/irqchip/irq-tegra.c | 20 ++++++++++++++++++--
>>>>>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
>>>>>> index e1f771c72fc4..851f88cef508 100644
>>>>>> --- a/drivers/irqchip/irq-tegra.c
>>>>>> +++ b/drivers/irqchip/irq-tegra.c
>>>>>> @@ -44,6 +44,7 @@ static unsigned int num_ictlrs;
>>>>>>   
>>>>>>   struct tegra_ictlr_soc {
>>>>>>   	unsigned int num_ictlrs;
>>>>>> +	bool supports_sc7;
>>>>>>   };
>>>>>>   
>>>>>>   static const struct tegra_ictlr_soc tegra20_ictlr_soc = {
>>>>>> @@ -56,6 +57,7 @@ static const struct tegra_ictlr_soc tegra30_ictlr_soc = {
>>>>>>   
>>>>>>   static const struct tegra_ictlr_soc tegra210_ictlr_soc = {
>>>>>>   	.num_ictlrs = 6,
>>>>>> +	.supports_sc7 = true,
>>>>>>   };
>>>>>>   
>>>>>>   static const struct of_device_id ictlr_matches[] = {
>>>>>> @@ -67,6 +69,7 @@ static const struct of_device_id ictlr_matches[] = {
>>>>>>   
>>>>>>   struct tegra_ictlr_info {
>>>>>>   	void __iomem *base[TEGRA_MAX_NUM_ICTLRS];
>>>>>> +	const struct tegra_ictlr_soc *soc;
>>>>>>   #ifdef CONFIG_PM_SLEEP
>>>>>>   	u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
>>>>>>   	u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
>>>>>> @@ -147,8 +150,20 @@ static int tegra_ictlr_suspend(void)
>>>>>>   		lic->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
>>>>>>   		lic->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
>>>>>>   
>>>>>> -		/* Disable COP interrupts */
>>>>>> -		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
>>>>>> +		/*
>>>>>> +		 * AVP/COP/BPMP-Lite is the Tegra boot processor.
>>>>>> +		 *
>>>>>> +		 * Tegra210 system suspend flow uses sc7entry firmware which
>>>>>> +		 * is executed by COP/BPMP and it includes disabling COP IRQ,
>>>>>> +		 * clamping CPU rail, turning off VDD_CPU, and preparing the
>>>>>> +		 * system to go to SC7/LP0.
>>>>>> +		 *
>>>>>> +		 * COP/BPMP wakes up when COP IRQ is triggered and runs
>>>>>> +		 * sc7entry-firmware. So need to keep COP interrupt enabled.
>>>>>> +		 */
>>>>>> +		if (!lic->soc->supports_sc7)
>>>>>> +			/* Disable COP interrupts if SC7 is not supported */  
>>>>> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
>>>>> doesn't sound correct to me. Something like 'firmware_sc7' should suit
>>>>> better here.  
>>>> If what you're saying is true, then the whole patch is wrong, and the
>>>> SC7 property should come from DT.  
>>> It should be safe to assume that all of existing Tegra210 devices use
>>> the firmware for SC7, hence I wouldn't say that the patch is entirely
>>> wrong. To me it's not entirely correct.  
>>
>> Yes, all existing Tegra210 platforms uses sc7 entry firmware for SC7 and 
>> AVP/COP IRQ need to be kept enabled as during suspend ATF triggers IRQ 
>> to COP for SC7 entry fw execution.

Okay, as I already wrote before, it looks to me that a more proper
solution should be to just remove everything related to COP from this
driver instead of adding custom quirks for T210.

The disabling / restoring of COP interrupts should be relevant only for
the multimedia firmware on older Tegra SoCs. That firmware won't be ever
supported in the upstream simply because NVIDIA abandoned the support
for older hardware in the downstream and because it is not possible due
to some legal weirdness (IIUC). The only variant for upstream is
reverse-engineering of hardware (not the firmware BLOB) and writing
proper opensource drivers for the upstream kernel, which we're already
doing and have success to a some extent.

> That's not the question. Dmitry says that the SC7 support is not a
> property of the SoC, but mostly a platform decision on whether the
> firmware supports SC7 or not.
> 
> To me, that's a clear indication that this should not be hardcoded in
> the driver, but instead obtained dynamically, via DT or otherwise.

We already have an nvidia,suspend-mode property in the device-tree of
the Power Management Controller node (all Tegra SoCs) which defines what
suspending type is supported by a particular board.

>>>>>> +			writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);  
>>>>> Secondly, I'm also not sure why COP interrupts need to be disabled for
>>>>> pre-T210 at all, since COP is unused. This looks to me like it was
>>>>> cut-n-pasted from downstream kernel without a good reason and could be
>>>>> simply removed.  
>>>> Please verify that this is actually the case. Tegra-2 definitely needed
>>>> some level of poking, and I'm not keen on changing anything there until
>>>> you (or someone else) has verified it on actual HW (see e307cc8941fc).  
>>> Tested on Tegra20 and Tegra30, LP1 suspend-resume works perfectly fine
>>> with all COP bits removed from the driver.
>>>
>>> AFAIK, the reason why downstream needed that disabling is that it uses
>>> proprietary firmware which is running on the COP and that firmware is
>>> usually a BLOB audio/video DEC-ENC driver which doesn't cleanup
>>> interrupts after itself. That firmware is not applicable for the
>>> upstream kernel, hence there is no need to care about it.
>>>  
>>>> Joseph, can you please shed some light here?  
>>
>> SC7 entry flow uses 3rd party ATF (arm-trusted FW) blob which is the
>> one that actually loads SC7 entry firmware and triggers IRQ to
>> AVP/COP which causes COP to wakeup and run SC7 entry FW.
>>
>> So when SC7 support is enabled, IRQ need to be kept enabled and when
>> SC7 FW starts execution, it will disable COP IRQ.
> 
> This looks like a lot of undocumented assumptions on what firmware
> does, as well as what firmware *is*. What I gather from this thread is
> that there is at least two versions of firmware (a "proprietary
> firmware" for "downstream kernels", and another one for mainline), and
> that they do different things.
> 
> Given that we cannot know what people actually run, I don't think we
> can safely remove anything unless this gets tested on the full spectrum
> of HW/FW combination.

I'm not sure whether multiple firmware variations exist in the wild for
Tegra210. Maybe Sowjanya or somebody else from NVIDIA could clarify. I
think there should be some efforts in regards to a fully opensource
firmware on Tegra210, but I'm not following it and have no idea about
the status.

You're right that there are multiple variants of suspend-resuming flow
on Tegra SoCs. The older 32bit Tegra SoC generations have a variety of
options in regards to suspend-resuming, including firmware-less variants
on platforms that are having kernel running in secure mode (dev boards,
most of Tegra20 consumer devices) and Trusted-Foundations firmware
variant for insecure platforms (consumer devices). And yes, vendor
firmware creates a lot of headache in regards to bringing support into
upstream because it usually does a lot of odd undocumented things which
may also vary depending on a firmware version (bootloader, etc) and it
also usually difficult to replace it with an opensource alternative due
to a crypto signing.

^ permalink raw reply

* [v5 2/2] gpio: aspeed: Add SGPIO driver
From: Hongwei Zhang @ 2019-07-22 20:36 UTC (permalink / raw)
  To: Linus Walleij, Andrew Jeffery, linux-gpio, Joel Stanley
  Cc: linux-aspeed, Bartosz Golaszewski, linux-kernel, linux-arm-kernel,
	Hongwei Zhang
In-Reply-To: <1563564291-9692-3-git-send-email-hongweiz@ami.com>

Hello Linus,

Thanks for your reviewing, please find my inline comment on why we group the 
("A", "B", "C", "D") etc. together at below. We will address other concerns 
separately.
--Hongwei

> From:	Linus Walleij <linus.walleij@linaro.org>
> Sent:	Saturday, July 20, 2019 3:37 AM
> To:	Hongwei Zhang
> Cc:	Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley; linux-aspeed@lists.ozlabs.org; Bartosz 
> Golaszewski; linux-kernel@vger.kernel.org; Linux ARM
> Subject:	Re: [v5 2/2] gpio: aspeed: Add SGPIO driver
> 
> Hi Hongwei,
> 
> thanks for your patch!
> 
> some comments and nitpicking below:
> 
> On Fri, Jul 19, 2019 at 9:25 PM Hongwei Zhang <hongweiz@ami.com> wrote:
> 
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang <hongweiz@ami.com>
> 
> > +// SPDX-License-Identifier: GPL-2.0+
> 
> I think the SPDX people prefer GPL-2.0-or-later
> 
> > +#include <linux/gpio.h>
> 
> Do not include this header in any new code using or providing GPIOs.
> 
> > +#include <linux/gpio/driver.h>
> 
> This should be enough.
> 
> > +/*
> > + * Note: The "value" register returns the input value when the GPIO is
> > + *      configured as an input.
> > + *
> > + *      The "rdata" register returns the output value when the GPIO is
> > + *      configured as an output.
> > + */
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > +       {
> > +               .val_regs = 0x0000,
> > +               .rdata_reg = 0x0070,
> > +               .irq_regs = 0x0004,
> > +               .names = { "A", "B", "C", "D" },
> > +       },
> > +       {
> > +               .val_regs = 0x001C,
> > +               .rdata_reg = 0x0074,
> > +               .irq_regs = 0x0020,
> > +               .names = { "E", "F", "G", "H" },
> > +       },
> > +       {
> > +               .val_regs = 0x0038,
> > +               .rdata_reg = 0x0078,
> > +               .irq_regs = 0x003C,
> > +               .names = { "I", "J" },
> > +       },
> > +};
> 
> I guess you have been over the reasons why this is one big GPIO chip instead of  10 individual gpio_chips?
> 
> It is usally better to have the individual chips, because it is easier to just cut down the code to handle 
> one instance and not having to offset around the different address ranges.
> 
> Even if they all have the same clock, the clocks are reference counted so it will just be referenced 10 
> times at most.
> 
> If they share a few common registers it is not good to split it though. So there may be a compelling 
> argument for keeping them all together.
> 

As you suspected it correctly, AST2500 utilizes all the 32 bits of the registers 
(data value, interrupt, etc...), such that using 8-bit bands
[7:0]/[15:8]/23:16]/[31:24] of GPIO_200H for SGPIO_A/B/C/D . 
so registering 10 gpiochip drivers separately will make code more 
complicated, for example gpio_200 register (data_value reg) has to be 
shared by 4 gpiochip instances, and the same is true for gpio204 (interrupt reg), 
and other more registers.
So we would prefer to keeping current implementation.

> > +/* This will be resolved at compile time */
> 
> I don't see why that matters.
> 
> > +static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
> > +                                    const struct aspeed_sgpio_bank *bank,
> > +                                    const enum aspeed_sgpio_reg reg)
> 
> You don't need inline. The compiler will inline it anyway if it see the need for it.
> 
> The only time we really use inline is in header files, where we want to point out that this function will be 
> inlined as there is no compiled code in header files.
> 
> > +#define GPIO_BANK(x)    ((x) >> 5)
> > +#define GPIO_OFFSET(x)  ((x) & 0x1f)
> > +#define GPIO_BIT(x)     BIT(GPIO_OFFSET(x))
> 
> OK seems fairly standard.
> 
> > +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int 
> > +offset) static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned 
> > +int offset, int val) static int aspeed_sgpio_dir_in(struct gpio_chip 
> > +*gc, unsigned int offset)
> 
> These are fairly standard.
> 
> > +static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int 
> > +offset, int val) {
> > +       struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > +       unsigned long flags;
> > +
> > +       spin_lock_irqsave(&gpio->lock, flags);
> > +       gpio->dir_in[GPIO_BANK(offset)] &= ~GPIO_BIT(offset);
> > +       spin_unlock_irqrestore(&gpio->lock, flags);
> > +
> > +       return 0;
> > +}
> 
> There is a bug here. You fail to write the "val" to the output line, which is the expected semantic of this 
> call.
> 
> > +static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned 
> > +int offset)
> 
> These are all very simple MMIO accessors.
> 
> If you made one gpio_chip per bank, you could just use gpio-mmio.c to control the lines by
> 
> select GPIO_GENERIC
> 
>         ret = bgpio_init(chip, dev, 4,
>                          base + GPIO_VAL_VALUE ,
>                          NULL,
>                          NULL,
>                          NULL,
>                          NULL,
>                          0);
> 
> The MMIO gpio library takes care of shadowing the direction and all.
> It also will implement get/set_multiple() for you for free.
> 
> So seriously consider making one gpio_chip per bank.
> 
> > +static inline void irqd_to_aspeed_sgpio_data(struct irq_data *d, 
> > +static void aspeed_sgpio_irq_ack(struct irq_data *d) static void 
> > +aspeed_sgpio_irq_set_mask(struct irq_data *d, bool set) static void 
> > +aspeed_sgpio_irq_mask(struct irq_data *d) static void 
> > +aspeed_sgpio_irq_unmask(struct irq_data *d) static int 
> > +aspeed_sgpio_set_type(struct irq_data *d, unsigned int type) static 
> > +void aspeed_sgpio_irq_handler(struct irq_desc *desc) {
> > +       struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> > +       struct irq_chip *ic = irq_desc_get_chip(desc);
> > +       struct aspeed_sgpio *data = gpiochip_get_data(gc);
> > +       unsigned int i, p, girq;
> > +       unsigned long reg;
> > +
> > +       chained_irq_enter(ic, desc);
> > +
> > +       for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> > +               const struct aspeed_sgpio_bank *bank = 
> > + &aspeed_sgpio_banks[i];
> > +
> > +               reg = ioread32(bank_reg(data, bank, reg_irq_status));
> > +
> > +               for_each_set_bit(p, &reg, 32) {
> > +                       girq = irq_find_mapping(gc->irq.domain, i * 32 + p);
> > +                       generic_handle_irq(girq);
> > +               }
> > +
> > +       }
> 
> This also gets really complex with one driver for all the banks.
> 
> > +       /* Disable IRQ and clear Interrupt status registers for all SPGIO Pins. */
> > +       for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
> 
> (...)
> > +static int __init aspeed_sgpio_probe(struct platform_device *pdev) {
> > +       struct aspeed_sgpio *gpio;
> > +       u32 nr_gpios, sgpio_freq, sgpio_clk_div;
> > +       int rc;
> > +       unsigned long apb_freq;
> > +
> > +       /* initialize allocated memory with zeros */
> 
> No need for this comment, developers know what "kzalloc" means.
> 
> > +       rc = of_property_read_u32(pdev->dev.of_node, "bus-frequency", &sgpio_freq);
> > +       if (rc < 0) {
> > +               dev_err(&pdev->dev, "Could not read bus-frequency property\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       gpio->pclk = devm_clk_get(&pdev->dev, NULL);
> > +       if (IS_ERR(gpio->pclk)) {
> > +               dev_err(&pdev->dev, "devm_clk_get failed\n");
> > +               return PTR_ERR(gpio->pclk);
> > +       }
> > +
> > +       apb_freq = clk_get_rate(gpio->pclk);
> > +
> > +       /*
> > +        * From the datasheet,
> > +        *      SGPIO period = 1/PCLK * 2 * (GPIO254[31:16] + 1)
> > +        *      period = 2 * (GPIO254[31:16] + 1) / PCLK
> > +        *      frequency = 1 / (2 * (GPIO254[31:16] + 1) / PCLK)
> > +        *      frequency = PCLK / (2 * (GPIO254[31:16] + 1))
> > +        *      frequency * 2 * (GPIO254[31:16] + 1) = PCLK
> > +        *      GPIO254[31:16] = PCLK / (frequency * 2) - 1
> > +        */
> > +       if (sgpio_freq == 0)
> > +               return -EINVAL;
> > +
> > +       sgpio_clk_div = (apb_freq / (sgpio_freq * 2)) - 1;
> > +
> > +       if (sgpio_clk_div > (1 << 16) - 1)
> > +               return -EINVAL;
> > +
> > +       iowrite32(FIELD_PREP(ASPEED_SGPIO_CLK_DIV_MASK, sgpio_clk_div) |
> > +                 FIELD_PREP(ASPEED_SGPIO_PINS_MASK, (nr_gpios / 8)) |
> > +                 ASPEED_SGPIO_ENABLE,
> > +                 gpio->base + ASPEED_SGPIO_CTRL);
> 
> This is a separate clock driver.
> 
> Break this out as a separate clk device that the other GPIOs grab.
> 
> Put this in drivers/clk/clk-aspeed-gpio.c or wherever appropriate with some reg = <0xnnnnnn54 4>;
> 
> Then let the GPIO driver grab this clock. This makes it possible to use a per-gpio-bank split of the GPIO 
> chips.
> 
> It looks a bit complicated but this will work so much better because the clock code is in the clock 
> subsystem and the GPIO is split up and becomes a very small driver since it can use gpio MMIO.
> 
> Yours,
> Linus Walleij

^ permalink raw reply

* Re: [PATCH v4 7/8] dt-bindings: arm: sunxi: add binding for Lichee Zero Plus core board
From: Maxime Ripard @ 2019-07-22 19:29 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: linux-arm-kernel, devicetree, Linus Walleij, linux-sunxi,
	linux-kernel, linux-gpio, Chen-Yu Tsai, Rob Herring, linux-clk
In-Reply-To: <BDF0C9F6-DD0D-4343-8E24-06A07055004C@aosc.io>

[-- Attachment #1: Type: text/plain, Size: 2094 bytes --]

On Sat, Jul 20, 2019 at 07:39:08PM +0800, Icenowy Zheng wrote:
>
>
> 于 2019年7月20日 GMT+08:00 下午6:13:18, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >On Sat, Jul 13, 2019 at 11:46:33AM +0800, Icenowy Zheng wrote:
> >> The Lichee Zero Plus is a core board made by Sipeed, with a microUSB
> >> connector on it, TF slot or WSON8 SD chip, optional eMMC or SPI
> >Flash.
> >> It has a gold finger connector for expansion, and UART is available
> >from
> >> reserved pins w/ 2.54mm pitch. The board can use either SoChip S3 or
> >> Allwinner V3L SoCs.
> >>
> >> Add the device tree binding of the basic version of the core board --
> >> w/o eMMC or SPI Flash, w/ TF slot or WSON8 SD, and use S3 SoC.
> >>
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >> No changes since v3.
> >>
> >> Patch introduced in v2.
> >>
> >>  Documentation/devicetree/bindings/arm/sunxi.yaml | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml
> >b/Documentation/devicetree/bindings/arm/sunxi.yaml
> >> index 000a00d12d6a..48c126a7a848 100644
> >> --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> >> @@ -353,6 +353,11 @@ properties:
> >>            - const: licheepi,licheepi-zero
> >>            - const: allwinner,sun8i-v3s
> >>
> >> +      - description: Lichee Zero Plus (with S3, without eMMC/SPI
> >Flash)
> >> +        items:
> >> +          - const: sipeed,lichee-zero-plus
> >> +          - const: allwinner,sun8i-s3
> >
> >If the S3 is just a rebranded V3, then we should have the v3 compatile
> >in that list too.
>
> S3 is V3 with copackaged DDR3 DRAM.
>
> It's pin incompatible w/ V3.

Does it matter though?

If the only thing that changes is the package, we're not manipulating
that, and any software that deals with the v3 can deal with the
s3. Which is what the compatible is about.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Marc Zyngier @ 2019-07-22 18:38 UTC (permalink / raw)
  To: Sowjanya Komatineni
  Cc: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
	linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
	sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
	linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <a2ecc3ad-b7e9-9398-d59b-c7d3fbbd10bb@nvidia.com>

On Mon, 22 Jul 2019 09:21:21 -0700
Sowjanya Komatineni <skomatineni@nvidia.com> wrote:

> On 7/22/19 3:57 AM, Dmitry Osipenko wrote:
> > 22.07.2019 13:13, Marc Zyngier пишет:  
> >> On 22/07/2019 10:54, Dmitry Osipenko wrote:  
> >>> 21.07.2019 22:40, Sowjanya Komatineni пишет:  
> >>>> Tegra210 platforms use sc7 entry firmware to program Tegra LP0/SC7 entry
> >>>> sequence and sc7 entry firmware is run from COP/BPMP-Lite.
> >>>>
> >>>> So, COP/BPMP-Lite still need IRQ function to finish SC7 suspend sequence
> >>>> for Tegra210.
> >>>>
> >>>> This patch has fix for leaving the COP IRQ enabled for Tegra210 during
> >>>> interrupt controller suspend operation.
> >>>>
> >>>> Acked-by: Thierry Reding <treding@nvidia.com>
> >>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> >>>> ---
> >>>>   drivers/irqchip/irq-tegra.c | 20 ++++++++++++++++++--
> >>>>   1 file changed, 18 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
> >>>> index e1f771c72fc4..851f88cef508 100644
> >>>> --- a/drivers/irqchip/irq-tegra.c
> >>>> +++ b/drivers/irqchip/irq-tegra.c
> >>>> @@ -44,6 +44,7 @@ static unsigned int num_ictlrs;
> >>>>   
> >>>>   struct tegra_ictlr_soc {
> >>>>   	unsigned int num_ictlrs;
> >>>> +	bool supports_sc7;
> >>>>   };
> >>>>   
> >>>>   static const struct tegra_ictlr_soc tegra20_ictlr_soc = {
> >>>> @@ -56,6 +57,7 @@ static const struct tegra_ictlr_soc tegra30_ictlr_soc = {
> >>>>   
> >>>>   static const struct tegra_ictlr_soc tegra210_ictlr_soc = {
> >>>>   	.num_ictlrs = 6,
> >>>> +	.supports_sc7 = true,
> >>>>   };
> >>>>   
> >>>>   static const struct of_device_id ictlr_matches[] = {
> >>>> @@ -67,6 +69,7 @@ static const struct of_device_id ictlr_matches[] = {
> >>>>   
> >>>>   struct tegra_ictlr_info {
> >>>>   	void __iomem *base[TEGRA_MAX_NUM_ICTLRS];
> >>>> +	const struct tegra_ictlr_soc *soc;
> >>>>   #ifdef CONFIG_PM_SLEEP
> >>>>   	u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
> >>>>   	u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
> >>>> @@ -147,8 +150,20 @@ static int tegra_ictlr_suspend(void)
> >>>>   		lic->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
> >>>>   		lic->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
> >>>>   
> >>>> -		/* Disable COP interrupts */
> >>>> -		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
> >>>> +		/*
> >>>> +		 * AVP/COP/BPMP-Lite is the Tegra boot processor.
> >>>> +		 *
> >>>> +		 * Tegra210 system suspend flow uses sc7entry firmware which
> >>>> +		 * is executed by COP/BPMP and it includes disabling COP IRQ,
> >>>> +		 * clamping CPU rail, turning off VDD_CPU, and preparing the
> >>>> +		 * system to go to SC7/LP0.
> >>>> +		 *
> >>>> +		 * COP/BPMP wakes up when COP IRQ is triggered and runs
> >>>> +		 * sc7entry-firmware. So need to keep COP interrupt enabled.
> >>>> +		 */
> >>>> +		if (!lic->soc->supports_sc7)
> >>>> +			/* Disable COP interrupts if SC7 is not supported */  
> >>> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
> >>> doesn't sound correct to me. Something like 'firmware_sc7' should suit
> >>> better here.  
> >> If what you're saying is true, then the whole patch is wrong, and the
> >> SC7 property should come from DT.  
> > It should be safe to assume that all of existing Tegra210 devices use
> > the firmware for SC7, hence I wouldn't say that the patch is entirely
> > wrong. To me it's not entirely correct.  
> 
> Yes, all existing Tegra210 platforms uses sc7 entry firmware for SC7 and 
> AVP/COP IRQ need to be kept enabled as during suspend ATF triggers IRQ 
> to COP for SC7 entry fw execution.

That's not the question. Dmitry says that the SC7 support is not a
property of the SoC, but mostly a platform decision on whether the
firmware supports SC7 or not.

To me, that's a clear indication that this should not be hardcoded in
the driver, but instead obtained dynamically, via DT or otherwise.

> 
> 
> >>>> +			writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);  
> >>> Secondly, I'm also not sure why COP interrupts need to be disabled for
> >>> pre-T210 at all, since COP is unused. This looks to me like it was
> >>> cut-n-pasted from downstream kernel without a good reason and could be
> >>> simply removed.  
> >> Please verify that this is actually the case. Tegra-2 definitely needed
> >> some level of poking, and I'm not keen on changing anything there until
> >> you (or someone else) has verified it on actual HW (see e307cc8941fc).  
> > Tested on Tegra20 and Tegra30, LP1 suspend-resume works perfectly fine
> > with all COP bits removed from the driver.
> >
> > AFAIK, the reason why downstream needed that disabling is that it uses
> > proprietary firmware which is running on the COP and that firmware is
> > usually a BLOB audio/video DEC-ENC driver which doesn't cleanup
> > interrupts after itself. That firmware is not applicable for the
> > upstream kernel, hence there is no need to care about it.
> >  
> >> Joseph, can you please shed some light here?  
> 
> SC7 entry flow uses 3rd party ATF (arm-trusted FW) blob which is the
> one that actually loads SC7 entry firmware and triggers IRQ to
> AVP/COP which causes COP to wakeup and run SC7 entry FW.
> 
> So when SC7 support is enabled, IRQ need to be kept enabled and when
> SC7 FW starts execution, it will disable COP IRQ.

This looks like a lot of undocumented assumptions on what firmware
does, as well as what firmware *is*. What I gather from this thread is
that there is at least two versions of firmware (a "proprietary
firmware" for "downstream kernels", and another one for mainline), and
that they do different things.

Given that we cannot know what people actually run, I don't think we
can safely remove anything unless this gets tested on the full spectrum
of HW/FW combination.

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* Re: [PATCH 00/18] ARM: Add minimal Raspberry Pi 4 support
From: Stefan Wahren @ 2019-07-22 18:10 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Matthias Brugger, Rob Herring, Mark Rutland, Linus Walleij,
	Michael Turquette, Stephen Boyd, Ulf Hansson, Adrian Hunter,
	bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Christoph Hellwig
In-Reply-To: <96113adb5fcab9c2f4bb5fa82b84ff5f9c07fd07.camel@suse.de>

Hi Nicolas,

[add Christoph]

Am 22.07.19 um 15:18 schrieb Nicolas Saenz Julienne:
> Hi Stefan,
> I rebased your bcm2711-initial branch on top of v5.3-rc1 and got this:
>
> [    2.307920] mmc1: new ultra high speed DDR50 SDHC card at address 59b4
> [    2.314972] mmcblk1: mmc1:59b4 USD   15.0 GiB
> [    2.320240]  mmcblk1: p1 p2
> [    2.324366] mmc0: new high speed SDIO card at address 0001
> [    2.365315] EXT4-fs (mmcblk1p2): mounted filesystem with ordered data mode. Opts: (null)
> [    2.373579] VFS: Mounted root (ext4 filesystem) readonly on device 179:2.
> [    2.381220] devtmpfs: mounted
> [    2.386618] Freeing unused kernel memory: 2048K
> [    2.394373] Run /sbin/init as init process
> [    2.644330] 8<--- cut here ---
> [    2.647439] Unable to handle kernel paging request at virtual address bb000000
> [    2.654795] pgd = (ptrval)
> [    2.657540] [bb000000] *pgd=00000000
> [    2.661172] Internal error: Oops: 2805 [#1] SMP ARM
> [    2.666122] Modules linked in:
> [    2.669225] CPU: 0 PID: 86 Comm: kworker/0:1H Not tainted 5.3.0-rc1-00020-g496d90bc72a9 #374
> [    2.677792] Hardware name: BCM2835
> [    2.681254] Workqueue: mmc_complete mmc_blk_mq_complete_work
> [    2.687002] PC is at v7_dma_inv_range+0x3c/0x54
> [    2.691602] LR is at __dma_page_dev_to_cpu+0x84/0x104
> [    2.696728] pc : [<c031cfec>]    lr : [<c0317468>]    psr: 80000013
> [    2.703089] sp : e8081e60  ip : ea059000  fp : 00000000
> [    2.708390] r10: 00000002  r9 : 00000002  r8 : e9fb9000
> [    2.713692] r7 : c1708cc4  r6 : c18cde80  r5 : 00001000  r4 : 00000000
> [    2.720316] r3 : 0000003f  r2 : 00000040  r1 : bb001000  r0 : bb000000
> [    2.726942] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> [    2.734185] Control: 10c5383d  Table: 0020406a  DAC: 00000051
> [    2.740017] Process kworker/0:1H (pid: 86, stack limit = 0x(ptrval))
> [    2.746466] Stack: (0xe8081e60 to 0xe8082000)
> [    2.750891] 1e60: c031d084 ea004480 e928f300 e8053014 00000002 c0319350 c0f015d0 e9196c10
> [    2.759197] 1e80: 00000002 c0317ac4 00000000 c124fc74 0000757d e97fb390 c0317a6c c1704c48
> [    2.767503] 1ea0: e92a60c4 00000000 e97fb2b0 e9782800 e9783800 c0cce770 00000000 ea004480
> [    2.775809] 1ec0: e928f300 e92a6008 e97fb200 c0cc8c68 e92a6008 e97fb200 c1704c48 e92a60c4
> [    2.784115] 1ee0: 00000000 c0cc9040 00000000 e4380000 00000000 ada3354f 28989000 e92a60d8
> [    2.792420] 1f00: e9286080 ea004280 ff7b0200 00000000 c1891c70 00000000 ea004280 c0360eb8
> [    2.800726] 1f20: ffffe000 ea004280 c1703d00 e9286080 ea004280 e9286094 c1703d00 ea004298
> [    2.809032] 1f40: ffffe000 00000008 ea004280 c0361bb4 ffffe000 c18915ac c124ea98 00000000
> [    2.817338] 1f60: ffffe000 e91fc380 e92d3b00 00000000 e8080000 e9286080 c0361b70 e90bfea4
> [    2.825643] 1f80: e91fc39c c0366dac 00000001 e92d3b00 c0366c64 00000000 00000000 00000000
> [    2.833948] 1fa0: 00000000 00000000 00000000 c03010e8 00000000 00000000 00000000 00000000
> [    2.842253] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.850559] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> [    2.858872] [<c031cfec>] (v7_dma_inv_range) from [<c0317468>] (__dma_page_dev_to_cpu+0x84/0x104)
> [    2.867797] [<c0317468>] (__dma_page_dev_to_cpu) from [<c0317ac4>] (arm_dma_unmap_sg+0x58/0x74)
> [    2.876635] [<c0317ac4>] (arm_dma_unmap_sg) from [<c0cce770>] (sdhci_post_req+0x5c/0x8c)
> [    2.884857] [<c0cce770>] (sdhci_post_req) from [<c0cc8c68>] (mmc_blk_mq_post_req+0x30/0xb4)
> [    2.893340] [<c0cc8c68>] (mmc_blk_mq_post_req) from [<c0cc9040>] (mmc_blk_mq_complete_prev_req.part.4+0x174/0x238)
> [    2.903855] [<c0cc9040>] (mmc_blk_mq_complete_prev_req.part.4) from [<c0360eb8>] (process_one_work+0x218/0x510)
> [    2.914104] [<c0360eb8>] (process_one_work) from [<c0361bb4>] (worker_thread+0x44/0x5c0)
> [    2.922323] [<c0361bb4>] (worker_thread) from [<c0366dac>] (kthread+0x148/0x150)
> [    2.929836] [<c0366dac>] (kthread) from [<c03010e8>] (ret_from_fork+0x14/0x2c)
> [    2.937168] Exception stack(0xe8081fb0 to 0xe8081ff8)
> [    2.942296] 1fa0:                                     00000000 00000000 00000000 00000000
> [    2.950601] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.958906] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    2.965623] Code: e1110003 e1c11003 1e071f3e e1500001 (3e070f36)
> [    2.971830] ---[ end trace 918fd7aaa4c1e4cc ]---
>
> I'm looking into it. Any ideas?

i rebased this series also and got this only on the RPi 4.

After reverting the following:

79a986721de dma-mapping: remove dma_max_pfn
7559d612dff0 mmc: core: let the dma map ops handle bouncing

This crash disappear, but wifi seems to be still broken.

Would be nice, if you can investigate further.

Regards
Stefan

>
> Regards,
> Nicolas
>

^ permalink raw reply

* Re: [PATCH 00/18] ARM: Add minimal Raspberry Pi 4 support
From: Eric Anholt @ 2019-07-22 18:01 UTC (permalink / raw)
  To: Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563774880-8061-1-git-send-email-wahrenst@gmx.net>

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

Stefan Wahren <wahrenst@gmx.net> writes:

> This series adds minimal support for the new Raspberry Pi 4, so we are able
> to login via debug UART.
>
> Patch 1-5:   Prepare platform and DTS for the new SoC BMC2711
> Patch 6-10:  Enable support for emmc2 on BCM2711
> Patch 11-12: Enable pinctrl for BCM2711
> Patch 13-17: Add Raspberry Pi 4 DTS support
> Patch 18:    Update MAINTAINERS
>
> Unfortunately the Raspberry Pi Foundation didn't released a
> peripheral documentation for the new SoC yet. So we only have a preliminary
> datasheet [1] and reduced schematics [2].
>
> Changes since RFC:
> - change BCM2838 -> BCM2711 as discussed in RFC
> - update MAINTAINERS accordingly
> - drop "spi: bcm2835: enable shared interrupt support" from series
> - squash all pinctrl-bcm2835 changes into one patch
> - introduce SoC specific clock registration as suggested by Florian
> - fix watchdog probing for Raspberry Pi 4
> - convert brcm,bcm2835.txt to json-schema
> - move VC4 node to bcm2835-common.dtsi
> - fallback to legacy pull config for Raspberry Pi 4
> - revert unintended change of mailbox in bcm283x.dtsi
> - add reference for arm64


I gave some comments on the bcm2835-common patch, but I think this is a
good start and would be worth merging to start bringing the board up,
even without a respin.

Series is:

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply

* Re: [PATCH 03/18] ARM: dts: bcm283x: Move BCM2835/6/7 specific to bcm2835-common.dtsi
From: Eric Anholt @ 2019-07-22 17:54 UTC (permalink / raw)
  To: Stefan Wahren, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563774880-8061-4-git-send-email-wahrenst@gmx.net>

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

Stefan Wahren <wahrenst@gmx.net> writes:

> As preparation we want all common BCM2711 + BCM2835/6/7 functions in
> bcm283x.dtsi and all BCM2835/6/7 specific in the new bcm2835-common.dtsi.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

For display:

We should probably move HDMI to bcm2835-common.dtsi as well, as the
block has very thoroughly changed.

The pixelvalves have also changed and will probably need to be separate
for 2711.

DSI, DPI, and VEC I belive are the same thing at the same address still.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply

* [PATCH 16/18] ARM: dts: Add minimal Raspberry Pi 4 support
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563815257-2648-1-git-send-email-wahrenst@gmx.net>

This adds minimal support for the new Raspberry Pi 4 without the
fancy stuff like GENET, PCIe, XHCI, 40 bit DMA and V3D. The RPi 4 is available
in 3 different variants (1, 2 and 4 GB RAM), so leave the memory size to zero
and let the bootloader take care of it.

Other differences to the Raspberry Pi 3:
- additional GIC 400 Interrupt controller
- new thermal IP and HWRNG
- additional MMC interface (emmc2)
- additional UART, I2C, SPI and PWM interfaces

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 arch/arm/boot/dts/Makefile            |   1 +
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 120 ++++++
 arch/arm/boot/dts/bcm2711.dtsi        | 667 ++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/bcm283x.dtsi        |   2 +-
 4 files changed, 789 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/bcm2711-rpi-4-b.dts
 create mode 100644 arch/arm/boot/dts/bcm2711.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9159fa2..031ca32 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -83,6 +83,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
 	bcm2837-rpi-3-b.dtb \
 	bcm2837-rpi-3-b-plus.dtb \
 	bcm2837-rpi-cm3-io3.dtb \
+	bcm2711-rpi-4-b.dtb \
 	bcm2835-rpi-zero.dtb \
 	bcm2835-rpi-zero-w.dtb
 dtb-$(CONFIG_ARCH_BCM_5301X) += \
diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
new file mode 100644
index 0000000..806cd7b
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+#include "bcm2711.dtsi"
+#include "bcm2835-rpi.dtsi"
+
+/ {
+	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
+	model = "Raspberry Pi 4 Model B";
+
+	chosen {
+		/* 8250 auxiliary UART instead of pl011 */
+		stdout-path = "serial1:115200n8";
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0 0 0x00000000>;
+	};
+
+	leds {
+		act {
+			gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
+		};
+
+		pwr {
+			label = "PWR";
+			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	wifi_pwrseq: wifi-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		reset-gpios = <&expgpio 1 GPIO_ACTIVE_LOW>;
+	};
+
+	sd_io_1v8_reg: sd_io_1v8_reg {
+		compatible = "regulator-gpio";
+		regulator-name = "vdd-sd-io";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+		regulator-settling-time-us = <5000>;
+		gpios = <&expgpio 4 GPIO_ACTIVE_HIGH>;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		status = "okay";
+	};
+};
+
+&firmware {
+	expgpio: gpio {
+		compatible = "raspberrypi,firmware-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-line-names = "BT_ON",
+				  "WL_ON",
+				  "PWR_LED_OFF",
+				  "GLOBAL_RESET",
+				  "VDD_SD_IO_SEL",
+				  "CAM_GPIO",
+				  "",
+				  "";
+		status = "okay";
+	};
+};
+
+&pwm1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm1_0_gpio40 &pwm1_1_gpio41>;
+	status = "okay";
+};
+
+/* SDHCI is used to control the SDIO for wireless */
+&sdhci {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_gpio34>;
+	bus-width = <4>;
+	non-removable;
+	mmc-pwrseq = <&wifi_pwrseq>;
+	status = "okay";
+
+	brcmf: wifi@1 {
+		reg = <1>;
+		compatible = "brcm,bcm4329-fmac";
+	};
+};
+
+/* EMMC2 is used to drive the SD card */
+&emmc2 {
+	vqmmc-supply = <&sd_io_1v8_reg>;
+	broken-cd;
+	status = "okay";
+};
+
+/* uart0 communicates with the BT module */
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_ctsrts_gpio30 &uart0_gpio32 &gpclk2_gpio43>;
+	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		max-speed = <2000000>;
+		shutdown-gpios = <&expgpio 0 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+/* uart1 is mapped to the pin header */
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_gpio14>;
+	status = "okay";
+};
+
+&vchiq {
+	interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
new file mode 100644
index 0000000..b05d7f3
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -0,0 +1,667 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "bcm283x.dtsi"
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/soc/bcm2835-pm.h>
+
+/ {
+	compatible = "brcm,bcm2711";
+
+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	interrupt-parent = <&gicv2>;
+
+	soc {
+		ranges = <0x7e000000  0x0 0xfe000000  0x01800000>,
+			 <0x7c000000  0x0 0xfc000000  0x02000000>,
+			 <0x40000000  0x0 0xff800000  0x00800000>;
+		/* Emulate a contiguous 30-bit address range for DMA */
+		dma-ranges = <0xc0000000  0x0 0x00000000  0x3c000000>;
+
+		local_intc: local_intc@40000000 {
+			compatible = "brcm,bcm2836-l1-intc";
+			reg = <0x40000000 0x100>;
+		};
+
+		gicv2: gic400@40041000 {
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			compatible = "arm,gic-400";
+			reg =	<0x40041000 0x1000>,
+				<0x40042000 0x2000>,
+				<0x40044000 0x2000>,
+				<0x40046000 0x2000>;
+		};
+
+		dma: dma@7e007000 {
+			compatible = "brcm,bcm2835-dma";
+			reg = <0x7e007000 0xb00>;
+			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
+				     /* DMA lite 7 - 10 */
+				     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "dma0",
+					  "dma1",
+					  "dma2",
+					  "dma3",
+					  "dma4",
+					  "dma5",
+					  "dma6",
+					  "dma7",
+					  "dma8",
+					  "dma9",
+					  "dma10";
+			#dma-cells = <1>;
+			brcm,dma-channel-mask = <0x07f5>;
+		};
+
+		pm: watchdog@7e100000 {
+			compatible = "brcm,bcm2835-pm", "brcm,bcm2835-pm-wdt";
+			#power-domain-cells = <1>;
+			#reset-cells = <1>;
+			reg = <0x7e100000 0x114>,
+			      <0x7e00a000 0x24>,
+			      <0x7ec11000 0x20>;
+			clocks = <&clocks BCM2835_CLOCK_V3D>,
+				 <&clocks BCM2835_CLOCK_PERI_IMAGE>,
+				 <&clocks BCM2835_CLOCK_H264>,
+				 <&clocks BCM2835_CLOCK_ISP>;
+			clock-names = "v3d", "peri_image", "h264", "isp";
+			system-power-controller;
+		};
+
+		rng@7e104000 {
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+
+			/* RNG is incompatible to brcm,bcm2835-rng */
+			status = "disabled";
+		};
+
+		uart2: serial@7e201400 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7e201400 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_UART>,
+				 <&clocks BCM2835_CLOCK_VPU>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		uart3: serial@7e201600 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7e201600 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_UART>,
+				 <&clocks BCM2835_CLOCK_VPU>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		uart4: serial@7e201800 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7e201800 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_UART>,
+				 <&clocks BCM2835_CLOCK_VPU>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		uart5: serial@7e201a00 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7e201a00 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_UART>,
+				 <&clocks BCM2835_CLOCK_VPU>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		spi@7e204000 {
+			reg = <0x7e204000 0x0200>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		spi3: spi@7e204600 {
+			compatible = "brcm,bcm2835-spi";
+			reg = <0x7e204600 0x0200>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi4: spi@7e204800 {
+			compatible = "brcm,bcm2835-spi";
+			reg = <0x7e204800 0x0200>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi5: spi@7e204a00 {
+			compatible = "brcm,bcm2835-spi";
+			reg = <0x7e204a00 0x0200>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spi6: spi@7e204c00 {
+			compatible = "brcm,bcm2835-spi";
+			reg = <0x7e204c00 0x0200>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@7e205600 {
+			compatible = "brcm,bcm2835-i2c";
+			reg = <0x7e205600 0x200>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c4: i2c@7e205800 {
+			compatible = "brcm,bcm2835-i2c";
+			reg = <0x7e205800 0x200>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c5: i2c@7e205a00 {
+			compatible = "brcm,bcm2835-i2c";
+			reg = <0x7e205a00 0x200>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		i2c6: i2c@7e205c00 {
+			compatible = "brcm,bcm2835-i2c";
+			reg = <0x7e205c00 0x200>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2835_CLOCK_VPU>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		pixelvalve@7e206000 {
+			interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		pixelvalve@7e207000 {
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		pwm1: pwm@7e20c800 {
+			compatible = "brcm,bcm2835-pwm";
+			reg = <0x7e20c800 0x28>;
+			clocks = <&clocks BCM2835_CLOCK_PWM>;
+			assigned-clocks = <&clocks BCM2835_CLOCK_PWM>;
+			assigned-clock-rates = <10000000>;
+			#pwm-cells = <2>;
+			status = "disabled";
+		};
+
+		emmc2: emmc2@7e340000 {
+			compatible = "brcm,bcm2711-emmc2";
+			reg = <0x7e340000 0x100>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clocks BCM2711_CLOCK_EMMC2>;
+			status = "disabled";
+		};
+
+		hvs@7e400000 {
+			interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
+		pixelvalve@7e807000 {
+			interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+		};
+	};
+
+	arm-pmu {
+		/*
+		 * N.B. the A72 PMU support only exists in arch/arm64, hence
+		 * the fallback to the A53 version.
+		 */
+		compatible = "arm,cortex-a72-pmu", "arm,cortex-a53-pmu";
+		interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	timer {
+		compatible = "arm,armv7-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+					  IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+					  IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+					  IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+					  IRQ_TYPE_LEVEL_LOW)>;
+		arm,cpu-registers-not-fw-configured;
+		always-on;
+	};
+
+	cpus: cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "brcm,bcm2836-smp"; // for ARM 32-bit
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x000000d8>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <1>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x000000e0>;
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <2>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x000000e8>;
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <3>;
+			enable-method = "spin-table";
+			cpu-release-addr = <0x0 0x000000f0>;
+		};
+	};
+};
+
+&clk_osc {
+	clock-frequency = <54000000>;
+};
+
+&clocks {
+	compatible = "brcm,bcm2711-cprman";
+};
+
+&cpu_thermal {
+	coefficients = <(-487) 410040>;
+};
+
+&dsi0 {
+	interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&dsi1 {
+	interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&gpio {
+	compatible = "brcm,bcm2711-gpio";
+	interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+
+	gpclk0_gpio49: gpclk0_gpio49 {
+		brcm,pins = <49>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+		brcm,pull = <BCM2835_PUD_OFF>;
+	};
+	gpclk1_gpio50: gpclk1_gpio50 {
+		brcm,pins = <50>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+		brcm,pull = <BCM2835_PUD_OFF>;
+	};
+	gpclk2_gpio51: gpclk2_gpio51 {
+		brcm,pins = <51>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+		brcm,pull = <BCM2835_PUD_OFF>;
+	};
+
+	i2c0_gpio46: i2c0_gpio46 {
+		brcm,pins = <46 47>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	i2c1_gpio46: i2c1_gpio46 {
+		brcm,pins = <46 47>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+	};
+	i2c3_gpio2: i2c3_gpio2 {
+		brcm,pins = <2 3>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c3_gpio4: i2c3_gpio4 {
+		brcm,pins = <4 5>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c4_gpio6: i2c4_gpio6 {
+		brcm,pins = <6 7>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c4_gpio8: i2c4_gpio8 {
+		brcm,pins = <8 9>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c5_gpio10: i2c5_gpio10 {
+		brcm,pins = <10 11>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c5_gpio12: i2c5_gpio12 {
+		brcm,pins = <12 13>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c6_gpio0: i2c6_gpio0 {
+		brcm,pins = <0 1>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c6_gpio22: i2c6_gpio22 {
+		brcm,pins = <22 23>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	i2c_slave_gpio8: i2c_slave_gpio8 {
+		brcm,pins = <8 9 10 11>;
+		brcm,function = <BCM2835_FSEL_ALT3>;
+	};
+
+	jtag_gpio48: jtag_gpio48 {
+		brcm,pins = <48 49 50 51 52 53>;
+		brcm,function = <BCM2835_FSEL_ALT4>;
+	};
+
+	mii_gpio28: mii_gpio28 {
+		brcm,pins = <28 29 30 31>;
+		brcm,function = <BCM2835_FSEL_ALT4>;
+	};
+	mii_gpio36: mii_gpio36 {
+		brcm,pins = <36 37 38 39>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+
+	pcm_gpio50: pcm_gpio50 {
+		brcm,pins = <50 51 52 53>;
+		brcm,function = <BCM2835_FSEL_ALT2>;
+	};
+
+	pwm0_0_gpio12: pwm0_0_gpio12 {
+		brcm,pins = <12>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	pwm0_0_gpio18: pwm0_0_gpio18 {
+		brcm,pins = <18>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	pwm1_0_gpio40: pwm1_0_gpio40 {
+		brcm,pins = <40>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	pwm0_1_gpio13: pwm0_1_gpio13 {
+		brcm,pins = <13>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	pwm0_1_gpio19: pwm0_1_gpio19 {
+		brcm,pins = <19>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	pwm1_1_gpio41: pwm1_1_gpio41 {
+		brcm,pins = <41>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	pwm0_1_gpio45: pwm0_1_gpio45 {
+		brcm,pins = <45>;
+		brcm,function = <BCM2835_FSEL_ALT0>;
+	};
+	pwm0_0_gpio52: pwm0_0_gpio52 {
+		brcm,pins = <52>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+		brcm,pull = <BCM2835_PUD_OFF>;
+	};
+	pwm0_1_gpio53: pwm0_1_gpio53 {
+		brcm,pins = <53>;
+		brcm,function = <BCM2835_FSEL_ALT1>;
+		brcm,pull = <BCM2835_PUD_OFF>;
+	};
+
+	/* The following group consists of:
+	 *  RGMII_START_STOP
+	 *  RGMII_RX_OK
+	 */
+	rgmii_gpio35: rgmii_gpio35 {
+		brcm,pins = <35 36>;
+		brcm,function = <BCM2835_FSEL_ALT4>;
+	};
+	rgmii_irq_gpio34: rgmii_irq_gpio34 {
+		brcm,pins = <34>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	rgmii_irq_gpio39: rgmii_irq_gpio39 {
+		brcm,pins = <39>;
+		brcm,function = <BCM2835_FSEL_ALT4>;
+	};
+	rgmii_mdio_gpio28: rgmii_mdio_gpio28 {
+		brcm,pins = <28 29>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	rgmii_mdio_gpio37: rgmii_mdio_gpio37 {
+		brcm,pins = <37 38>;
+		brcm,function = <BCM2835_FSEL_ALT4>;
+	};
+
+	spi0_gpio46: spi0_gpio46 {
+		brcm,pins = <46 47 48 49>;
+		brcm,function = <BCM2835_FSEL_ALT2>;
+	};
+	spi2_gpio46: spi2_gpio46 {
+		brcm,pins = <46 47 48 49 50>;
+		brcm,function = <BCM2835_FSEL_ALT5>;
+	};
+	spi3_gpio0: spi3_gpio0 {
+		brcm,pins = <0 1 2 3>;
+		brcm,function = <BCM2835_FSEL_ALT3>;
+	};
+	spi4_gpio4: spi4_gpio4 {
+		brcm,pins = <4 5 6 7>;
+		brcm,function = <BCM2835_FSEL_ALT3>;
+	};
+	spi5_gpio12: spi5_gpio12 {
+		brcm,pins = <12 13 14 15>;
+		brcm,function = <BCM2835_FSEL_ALT3>;
+	};
+	spi6_gpio18: spi6_gpio18 {
+		brcm,pins = <18 19 20 21>;
+		brcm,function = <BCM2835_FSEL_ALT3>;
+	};
+
+	uart2_gpio0: uart2_gpio0 {
+		pin-tx {
+			brcm,pins = <0>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+		pin-rx {
+			brcm,pins = <1>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+	};
+	uart2_ctsrts_gpio2: uart2_ctsrts_gpio2 {
+		pin-cts {
+			brcm,pins = <2>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+		pin-rts {
+			brcm,pins = <3>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+	};
+	uart3_gpio4: uart3_gpio4 {
+		pin-tx {
+			brcm,pins = <4>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+		pin-rx {
+			brcm,pins = <5>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+	};
+	uart3_ctsrts_gpio6: uart3_ctsrts_gpio6 {
+		pin-cts {
+			brcm,pins = <6>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+		pin-rts {
+			brcm,pins = <7>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+	};
+	uart4_gpio8: uart4_gpio8 {
+		pin-tx {
+			brcm,pins = <8>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+		pin-rx {
+			brcm,pins = <9>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+	};
+	uart4_ctsrts_gpio10: uart4_ctsrts_gpio10 {
+		pin-cts {
+			brcm,pins = <10>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+		pin-rts {
+			brcm,pins = <11>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+	};
+	uart5_gpio12: uart5_gpio12 {
+		pin-tx {
+			brcm,pins = <12>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+		pin-rx {
+			brcm,pins = <13>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+	};
+	uart5_ctsrts_gpio14: uart5_ctsrts_gpio14 {
+		pin-cts {
+			brcm,pins = <14>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_UP>;
+		};
+		pin-rts {
+			brcm,pins = <15>;
+			brcm,function = <BCM2835_FSEL_ALT4>;
+			brcm,pull = <BCM2835_PUD_OFF>;
+		};
+	};
+};
+
+&vec {
+	interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&usb {
+	interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&hdmi {
+	interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
+		     <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart1 {
+	interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&spi1 {
+	interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&spi2 {
+	interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&sdhci {
+	interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2c0 {
+	interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2c1 {
+	interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&i2c2 {
+	interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&mailbox {
+	interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&sdhost {
+	interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart0 {
+	interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&txp {
+	interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index 4329b17..2eea7b7 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -53,7 +53,7 @@
 		#address-cells = <1>;
 		#size-cells = <1>;

-		txp@7e004000 {
+		txp: txp@7e004000 {
 			compatible = "brcm,bcm2835-txp";
 			reg = <0x7e004000 0x20>;
 			interrupts = <1 11>;
--
2.7.4


^ permalink raw reply related

* [PATCH 17/18] arm64: dts: broadcom: Add reference to RPi 4 B
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563815257-2648-1-git-send-email-wahrenst@gmx.net>

This adds a reference to the dts of the Raspberry Pi 4 B,
so we don't need to maintain the content in arm64.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 arch/arm64/boot/dts/broadcom/Makefile            | 3 ++-
 arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dts

diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
index d1d31cc..cb7de8d 100644
--- a/arch/arm64/boot/dts/broadcom/Makefile
+++ b/arch/arm64/boot/dts/broadcom/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
-dtb-$(CONFIG_ARCH_BCM2835) += bcm2837-rpi-3-a-plus.dtb \
+dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-4-b.dtb \
+			      bcm2837-rpi-3-a-plus.dtb \
 			      bcm2837-rpi-3-b.dtb \
 			      bcm2837-rpi-3-b-plus.dtb \
 			      bcm2837-rpi-cm3-io3.dtb
diff --git a/arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dts
new file mode 100644
index 0000000..d24c536
--- /dev/null
+++ b/arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dts
@@ -0,0 +1,2 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "arm/bcm2711-rpi-4-b.dts"
--
2.7.4


^ permalink raw reply related

* [PATCH 18/18] MAINTAINERS: Add BCM2711 to BCM2835 ARCH
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563815257-2648-1-git-send-email-wahrenst@gmx.net>

Clarify that BCM2711 belongs to the BCM2835 ARCH.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c6fa7e8..ef75b7b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3137,7 +3137,7 @@ N:	bcm216*
 N:	kona
 F:	arch/arm/mach-bcm/

-BROADCOM BCM2835 ARM ARCHITECTURE
+BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
 M:	Eric Anholt <eric@anholt.net>
 M:	Stefan Wahren <wahrenst@gmx.net>
 L:	bcm-kernel-feedback-list@broadcom.com
@@ -3145,6 +3145,7 @@ L:	linux-rpi-kernel@lists.infradead.org (moderated for non-subscribers)
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 T:	git git://github.com/anholt/linux
 S:	Maintained
+N:	bcm2711
 N:	bcm2835
 F:	drivers/staging/vc04_services

--
2.7.4


^ permalink raw reply related

* [PATCH 15/18] ARM: bcm2835: Add bcm2711 compatible string
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563815257-2648-1-git-send-email-wahrenst@gmx.net>

This adds support for the new BCM2711 SoC found on the Raspberry Pi 4.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 arch/arm/mach-bcm/board_bcm2835.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
index d2b31a9..c09cf25 100644
--- a/arch/arm/mach-bcm/board_bcm2835.c
+++ b/arch/arm/mach-bcm/board_bcm2835.c
@@ -17,6 +17,7 @@ static const char * const bcm2835_compat[] = {
 	"brcm,bcm2835",
 #endif
 #ifdef CONFIG_ARCH_MULTI_V7
+	"brcm,bcm2711",
 	"brcm,bcm2836",
 	"brcm,bcm2837",
 #endif
--
2.7.4


^ permalink raw reply related

* [PATCH 13/18] dt-bindings: arm: Convert BCM2835 board/soc bindings to json-schema
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren

Convert the BCM2835/6/7 SoC bindings to DT schema format using json-schema.
All the other Broadcom boards are maintained by Florian Fainelli.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../devicetree/bindings/arm/bcm/bcm2835.yaml       | 46 +++++++++++++++
 .../devicetree/bindings/arm/bcm/brcm,bcm2835.txt   | 67 ----------------------
 2 files changed, 46 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
 delete mode 100644 Documentation/devicetree/bindings/arm/bcm/brcm,bcm2835.txt

diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
new file mode 100644
index 0000000..1a4be26
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/bcm/bcm2835.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM2711/BCM2835 Platforms Device Tree Bindings
+
+maintainers:
+  - Eric Anholt <eric@anholt.net>
+  - Stefan Wahren <wahrenst@gmx.net>
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+      - description: BCM2835 based Boards
+        items:
+          - enum:
+              - raspberrypi,model-a
+              - raspberrypi,model-a-plus
+              - raspberrypi,model-b
+              - raspberrypi,model-b-i2c0  # Raspberry Pi Model B (no P5)
+              - raspberrypi,model-b-rev2
+              - raspberrypi,model-b-plus
+              - raspberrypi,compute-module
+              - raspberrypi,model-zero
+              - raspberrypi,model-zero-w
+          - const: brcm,bcm2835
+
+      - description: BCM2836 based Boards
+        items:
+          - enum:
+              - raspberrypi,2-model-b
+
+      - description: BCM2837 based Boards
+        items:
+          - enum:
+              - raspberrypi,3-model-a-plus
+              - raspberrypi,3-model-b
+              - raspberrypi,3-model-b-plus
+              - raspberrypi,3-compute-module
+              - raspberrypi,3-compute-module-lite
+
+...
diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm2835.txt b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm2835.txt
deleted file mode 100644
index 245328f..0000000
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm2835.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Broadcom BCM2835 device tree bindings
--------------------------------------------
-
-Raspberry Pi Model A
-Required root node properties:
-compatible = "raspberrypi,model-a", "brcm,bcm2835";
-
-Raspberry Pi Model A+
-Required root node properties:
-compatible = "raspberrypi,model-a-plus", "brcm,bcm2835";
-
-Raspberry Pi Model B
-Required root node properties:
-compatible = "raspberrypi,model-b", "brcm,bcm2835";
-
-Raspberry Pi Model B (no P5)
-early model B with I2C0 rather than I2C1 routed to the expansion header
-Required root node properties:
-compatible = "raspberrypi,model-b-i2c0", "brcm,bcm2835";
-
-Raspberry Pi Model B rev2
-Required root node properties:
-compatible = "raspberrypi,model-b-rev2", "brcm,bcm2835";
-
-Raspberry Pi Model B+
-Required root node properties:
-compatible = "raspberrypi,model-b-plus", "brcm,bcm2835";
-
-Raspberry Pi 2 Model B
-Required root node properties:
-compatible = "raspberrypi,2-model-b", "brcm,bcm2836";
-
-Raspberry Pi 3 Model A+
-Required root node properties:
-compatible = "raspberrypi,3-model-a-plus", "brcm,bcm2837";
-
-Raspberry Pi 3 Model B
-Required root node properties:
-compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
-
-Raspberry Pi 3 Model B+
-Required root node properties:
-compatible = "raspberrypi,3-model-b-plus", "brcm,bcm2837";
-
-Raspberry Pi Compute Module
-Required root node properties:
-compatible = "raspberrypi,compute-module", "brcm,bcm2835";
-
-Raspberry Pi Compute Module 3
-Required root node properties:
-compatible = "raspberrypi,3-compute-module", "brcm,bcm2837";
-
-Raspberry Pi Compute Module 3 Lite
-Required root node properties:
-compatible = "raspberrypi,3-compute-module-lite", "brcm,bcm2837";
-
-Raspberry Pi Zero
-Required root node properties:
-compatible = "raspberrypi,model-zero", "brcm,bcm2835";
-
-Raspberry Pi Zero W
-Required root node properties:
-compatible = "raspberrypi,model-zero-w", "brcm,bcm2835";
-
-Generic BCM2835 board
-Required root node properties:
-compatible = "brcm,bcm2835";
--
2.7.4


^ permalink raw reply related

* [PATCH 14/18] dt-bindings: arm: bcm2835: Add Raspberry Pi 4 to DT schema
From: Stefan Wahren @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Eric Anholt, Florian Fainelli, Ray Jui, Scott Branden,
	Nicolas Saenz Julienne, Matthias Brugger, Rob Herring,
	Mark Rutland, Linus Walleij, Michael Turquette, Stephen Boyd,
	Ulf Hansson, Adrian Hunter
  Cc: bcm-kernel-feedback-list, linux-arm-kernel, linux-rpi-kernel,
	linux-gpio, linux-mmc, Stefan Wahren
In-Reply-To: <1563815257-2648-1-git-send-email-wahrenst@gmx.net>

Add new Raspberry Pi 4 to DT schema.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
index 1a4be26..8c3bbad 100644
--- a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
@@ -15,6 +15,11 @@ properties:
     const: '/'
   compatible:
     oneOf:
+      - description: BCM2711 based Boards
+        items:
+          - enum:
+              - raspberrypi,4-model-b
+
       - description: BCM2835 based Boards
         items:
           - enum:
--
2.7.4


^ permalink raw reply related

* Re: [PATCH v4 5/8] clk: sunxi-ng: v3s: add Allwinner V3 support
From: Rob Herring @ 2019-07-22 17:07 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Maxime Ripard, Chen-Yu Tsai, Linus Walleij, devicetree,
	linux-arm-kernel, linux-kernel, linux-clk, linux-gpio,
	linux-sunxi, Icenowy Zheng
In-Reply-To: <20190713034634.44585-6-icenowy@aosc.io>

On Sat, 13 Jul 2019 11:46:31 +0800, Icenowy Zheng wrote:
> Allwinner V3 has the same main die with V3s, but with more pins wired.
> There's a I2S bus on V3 that is not available on V3s.
> 
> Add the V3-only peripheral's clocks and reset to the V3s CCU driver,
> bound to a new V3 compatible string. The driver name is not changed
> because it's part of the device tree binding (the header file name).
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
> Changes in v4:
> - Add the missing MMC2 clock slices.
> 
> No changes in v3/v2.
> 
>  drivers/clk/sunxi-ng/ccu-sun8i-v3s.c      | 228 +++++++++++++++++++++-
>  drivers/clk/sunxi-ng/ccu-sun8i-v3s.h      |   2 +-
>  include/dt-bindings/clock/sun8i-v3s-ccu.h |   4 +
>  include/dt-bindings/reset/sun8i-v3s-ccu.h |   3 +
>  4 files changed, 234 insertions(+), 3 deletions(-)
> 

Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox