* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-22 16:01 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <5cb20f52-884a-b921-c904-ebf244092318@yadro.com>
On Thu, Aug 22, 2019 at 05:36:21PM +0300, Alexander Amelkin wrote:
> 21.08.2019 21:10, Guenter Roeck wrote:
> > On Wed, Aug 21, 2019 at 08:42:24PM +0300, Alexander Amelkin wrote:
> >> 21.08.2019 19:32, Guenter Roeck wrote:
> >>> On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
> >>>> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> >>>> to clear out boot code source and re-enable access to the primary SPI flash
> >>>> chip while booted via wdt2 from the alternate chip.
> >>>>
> >>>> AST2400 datasheet says:
> >>>> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> >>>> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> >>>> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> >>>> register WDT30.bit[1]."
> >>> Is there reason to not do this automatically when loading the module
> >>> in alt-boot mode ? What means does userspace have to determine if CS0
> >>> or CS1 is active at any given time ? If there is reason to ever have CS1
> >>> active instead of CS0, what means would userspace have to enable it ?
> >> Yes, there is. The driver is loaded long before the filesystems are mounted.
> >> The filesystems, in the event of alternate/recovery boot, need to be mounted
> >> from the same chip that the kernel was booted. For one reason because the main
> >> chip at CS0 is most probably corrupt. If you clear that bit when driver is
> >> loaded, your software will not know that and will try to mount the wrong
> >> filesystems. The whole idea of ASPEED's switching chipselects is to have
> >> identical firmware in both chips, without the need to process the alternate
> >> boot state in any way except for indicating a successful boot and restoring
> >> access to CS0 when needed.
> >>
> >> The userspace can read bootstatus sysfs node to determine if an alternate
> >> boot has occured.
> >>
> >> With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot
> >> from the primary flash chip (at CS0) and disable the watchdog to indicate a
> >> successful boot. When that happens, both CS0 and CS1 controls? get routed in
> >> hardware to CS1 line, making the primary flash chip inaccessible. Depending
> >> on the architecture of the user-space software, it may choose to re-enable
> >> access to the primary chip via CS0 at different times. There must be a way to do so.
> >>
> > So by activating cs0, userspace would essentially pull its own root file system
> > from underneath itself ?
>
> Exactly. That's why for alternate boot the firmware would usually copy
> all filesystems to memory and mount from there. Some embedded systems
> do that always, regardless of which chip they boot from.
>
That is different, though, to what you said earlier. Linux would then start
with a clean file system, and not need access to the file system in cs1 at all.
Clearing the flag when starting the driver would then be ok.
> However, to be able to recover the main flash chip, the system needs CS0
> to function as such (not as CS1). That's why this control is needed.
>
If what you said is correct, not really. It should be fine and create more
predictive behavior if the probe function selects cs0 automatically.
Guenter
> As Ivan mentioned, for AST2500 and the upcoming AST2600 the behavior
> is slightly different. They don't just connect both CS controls to CS1 but instead
> swap them so the primary chip becomes secondary from the software point
> of view. The means to restore the normal wiring may still be needed.
>
> >
> >> This code most probably adds nothing at the assembly level.
> >>
> > That seems quite unlikely. Please demonstrate.
>
> Yes, you were right. It adds 7 instructions. We'll drop the check.
> It's just my DO-178 background, I add 'robustness' checks everywhere.
>
> >>>> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> >>>> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> >>>> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> >>> The variable reflects the _boot status_. It should not change after booting.
> >> Is there any documentation that dictates that? All I could find is
> >>
> >> "bootstatus: status of the device after booting". That doesn't look to me like it absolutely can not change to reflect the updated status (that is, to reflect that the originally set up alternate CS routing has been reset to normal).
> >>
> > You choose to interpret "after booting" in a kind of novel way,
> > which I find a bit disturbing. I am not really sure how else to
> > describe "boot status" in a way that does not permit such
> > reinterpratation of the term.
>
> How about "Reflects reasons that caused a reboot, remains constant until the next boot" ?
>
> > On top of that, how specifically would "WDIOF_EXTERN1" reflect
> > what you claim it does ? Not only you are hijacking bootstatus9
> > (which is supposed to describe the reason for a reboot), you
> > are also hijacking WDIOF_EXTERN1. That seems highly arbitrary
> > to me, and is not really how an API/ABI should be used.
>
> We used WDIOF_EXTERN1 because:
>
> 1. We thought that bootstatus _can_ change
>
> 2. We thought that adding extra bits wouldn't be appreciated
>
> Now as you clarified that assumption 1 was wrong we are going to implement status as I proposed earlier:
>
> >
> >> I think we could make 'access_cs0' readable instead, so it could report the
> >> current state of the boot code selection bit. Reverted, I suppose. That
> >> way 'access_cs0' would report 1 after 1 has been written to it (it wouldn't
> >> be possible to write a zero).
>
> With best regards,
> Alexander Amelkin,
> BIOS/BMC Team Lead, YADRO
> https://yadro.com
>
>
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Alexander Amelkin @ 2019-08-22 14:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821181008.GB15127@roeck-us.net>
21.08.2019 21:10, Guenter Roeck wrote:
> On Wed, Aug 21, 2019 at 08:42:24PM +0300, Alexander Amelkin wrote:
>> 21.08.2019 19:32, Guenter Roeck wrote:
>>> On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
>>>> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
>>>> to clear out boot code source and re-enable access to the primary SPI flash
>>>> chip while booted via wdt2 from the alternate chip.
>>>>
>>>> AST2400 datasheet says:
>>>> "In the 2nd flash booting mode, all the address mapping to CS0# would be
>>>> re-directed to CS1#. And CS0# is not accessable under this mode. To access
>>>> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
>>>> register WDT30.bit[1]."
>>> Is there reason to not do this automatically when loading the module
>>> in alt-boot mode ? What means does userspace have to determine if CS0
>>> or CS1 is active at any given time ? If there is reason to ever have CS1
>>> active instead of CS0, what means would userspace have to enable it ?
>> Yes, there is. The driver is loaded long before the filesystems are mounted.
>> The filesystems, in the event of alternate/recovery boot, need to be mounted
>> from the same chip that the kernel was booted. For one reason because the main
>> chip at CS0 is most probably corrupt. If you clear that bit when driver is
>> loaded, your software will not know that and will try to mount the wrong
>> filesystems. The whole idea of ASPEED's switching chipselects is to have
>> identical firmware in both chips, without the need to process the alternate
>> boot state in any way except for indicating a successful boot and restoring
>> access to CS0 when needed.
>>
>> The userspace can read bootstatus sysfs node to determine if an alternate
>> boot has occured.
>>
>> With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot
>> from the primary flash chip (at CS0) and disable the watchdog to indicate a
>> successful boot. When that happens, both CS0 and CS1 controls? get routed in
>> hardware to CS1 line, making the primary flash chip inaccessible. Depending
>> on the architecture of the user-space software, it may choose to re-enable
>> access to the primary chip via CS0 at different times. There must be a way to do so.
>>
> So by activating cs0, userspace would essentially pull its own root file system
> from underneath itself ?
Exactly. That's why for alternate boot the firmware would usually copy
all filesystems to memory and mount from there. Some embedded systems
do that always, regardless of which chip they boot from.
However, to be able to recover the main flash chip, the system needs CS0
to function as such (not as CS1). That's why this control is needed.
As Ivan mentioned, for AST2500 and the upcoming AST2600 the behavior
is slightly different. They don't just connect both CS controls to CS1 but instead
swap them so the primary chip becomes secondary from the software point
of view. The means to restore the normal wiring may still be needed.
>
>> This code most probably adds nothing at the assembly level.
>>
> That seems quite unlikely. Please demonstrate.
Yes, you were right. It adds 7 instructions. We'll drop the check.
It's just my DO-178 background, I add 'robustness' checks everywhere.
>>>> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
>>>> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
>>>> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
>>> The variable reflects the _boot status_. It should not change after booting.
>> Is there any documentation that dictates that? All I could find is
>>
>> "bootstatus: status of the device after booting". That doesn't look to me like it absolutely can not change to reflect the updated status (that is, to reflect that the originally set up alternate CS routing has been reset to normal).
>>
> You choose to interpret "after booting" in a kind of novel way,
> which I find a bit disturbing. I am not really sure how else to
> describe "boot status" in a way that does not permit such
> reinterpratation of the term.
How about "Reflects reasons that caused a reboot, remains constant until the next boot" ?
> On top of that, how specifically would "WDIOF_EXTERN1" reflect
> what you claim it does ? Not only you are hijacking bootstatus9
> (which is supposed to describe the reason for a reboot), you
> are also hijacking WDIOF_EXTERN1. That seems highly arbitrary
> to me, and is not really how an API/ABI should be used.
We used WDIOF_EXTERN1 because:
1. We thought that bootstatus _can_ change
2. We thought that adding extra bits wouldn't be appreciated
Now as you clarified that assumption 1 was wrong we are going to implement status as I proposed earlier:
>
>> I think we could make 'access_cs0' readable instead, so it could report the
>> current state of the boot code selection bit. Reverted, I suppose. That
>> way 'access_cs0' would report 1 after 1 has been written to it (it wouldn't
>> be possible to write a zero).
With best regards,
Alexander Amelkin,
BIOS/BMC Team Lead, YADRO
https://yadro.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190822/af7bb64c/attachment-0001.sig>
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Ivan Mikhaylov @ 2019-08-22 14:24 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190822135528.GB8144@roeck-us.net>
On Thu, 2019-08-22 at 06:55 -0700, Guenter Roeck wrote:
> On Thu, Aug 22, 2019 at 12:15:20PM +0300, Ivan Mikhaylov wrote:
> > On Wed, 2019-08-21 at 09:32 -0700, Guenter Roeck wrote:
> > > > + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> > > > + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> > > > + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> > >
> > > The variable reflects the _boot status_. It should not change after
> > > booting.
> >
> > Okay, then perhaps may we set 'status' handler for watchdog device and
> > check
> > 'status' file? Right now 'bootstatus' and 'status' are same because there is
> > no
> > handler for 'status'.
> >
>
> You would still have to redefine one of the status bits to mean something
> driver specific. You would also still have two different flags to read
> and control cs0 - to read the status, you would read an ioctl (or the
> status sysfs attribute), to write it you would write into access_cs0.
>
> I guess I must be missing something. What is the problem with using
> access_cs0 for both ?
>
> Guenter
>
There is no problem, I'll do that way, thanks!
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-22 13:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <a022c0590f0fbf22cc8476b5ef3f1c22746429ac.camel@yadro.com>
On Thu, Aug 22, 2019 at 12:15:20PM +0300, Ivan Mikhaylov wrote:
> On Wed, 2019-08-21 at 09:32 -0700, Guenter Roeck wrote:
> >
> > > + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> > > + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> > > + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> >
> > The variable reflects the _boot status_. It should not change after booting.
>
> Okay, then perhaps may we set 'status' handler for watchdog device and check
> 'status' file? Right now 'bootstatus' and 'status' are same because there is no
> handler for 'status'.
>
You would still have to redefine one of the status bits to mean something
driver specific. You would also still have two different flags to read
and control cs0 - to read the status, you would read an ioctl (or the
status sysfs attribute), to write it you would write into access_cs0.
I guess I must be missing something. What is the problem with using
access_cs0 for both ?
Guenter
> > > +
> > > + return size;
> > > +}
> > > +static DEVICE_ATTR_WO(access_cs0);
> > > +
> > > +static struct attribute *bswitch_attrs[] = {
> > > + &dev_attr_access_cs0.attr,
> > > + NULL
> > > +};
> > > +ATTRIBUTE_GROUPS(bswitch);
> > > +
> > > static const struct watchdog_ops aspeed_wdt_ops = {
> > > .start = aspeed_wdt_start,
> > > .stop = aspeed_wdt_stop,
> > > @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device
> > > *pdev)
> > >
> > > wdt->ctrl = WDT_CTRL_1MHZ_CLK;
> > >
> > > + if (of_property_read_bool(np, "aspeed,alt-boot"))
> > > + wdt->wdd.groups = bswitch_groups;
> > > +
> > Why does this have to be separate to the existing evaluation of
> > aspeed,alt-boot, and why does the existing code not work ?
> >
> > Also, is it guaranteed that this does not interfer with existing
> > support for alt-boot ?
>
> It doesn't, it just provides for ast2400 switch to cs0 at side 1(cs1). Problem
> is that only one flash chip(side 1/cs1) is accessible on alternate boot, there
> is citation from the documentation in commit body. So if by some reason side 0
> is corrupted, need to switch into alternate boot to cs1, do the load from it,
> drop that bit to make side 0 accessible and do the flash of first side. On
> ast2500/2600 this problem is solved already, in alternate boot there both flash
> chips are present. It's additional requirement for alternate boot on ast2400, to
> make the possibility to access at all side 0 flash chip after we boot to the
> alternate side.
>
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Ulf Hansson @ 2019-08-22 12:14 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <c78d4c45-477b-4078-b269-aec72571c8cd@www.fastmail.com>
On Thu, 22 Aug 2019 at 13:55, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 22 Aug 2019, at 21:15, Ulf Hansson wrote:
> > On Thu, 15 Aug 2019 at 07:37, Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > >
> > >
> > > On Thu, 15 Aug 2019, at 15:06, Joel Stanley wrote:
> > > > On Wed, 7 Aug 2019 at 00:38, Andrew Jeffery <andrew@aj.id.au> wrote:
> > > > >
> > > > > The ASPEED SD/SDIO/MMC controller exposes two slots implementing the
> > > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> > > > > data bus if only a single slot is enabled.
> > > > >
> > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > >
> > > > Reviewed-by: Joel Stanley <joel@jms.id.au>
> > > >
> > > > Two minor comments below.
> > > >
> > > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > > > > @@ -0,0 +1,105 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > >
> > > > No "Copyright IBM" ?
> > >
> > > I'm going rogue.
> > >
> > > That reminds me I should chase up where we got to with the binding
> > > licensing.
> > >
> > > >
> > > > > +%YAML 1.2
> > > > > +---
> > > >
> > > > > +
> > > > > +examples:
> > > > > + - |
> > > > > + #include <dt-bindings/clock/aspeed-clock.h>
> > > > > + sdc at 1e740000 {
> > > > > + compatible = "aspeed,ast2500-sd-controller";
> > > > > + reg = <0x1e740000 0x100>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <1>;
> > > > > + ranges = <0 0x1e740000 0x10000>;
> > > >
> > > > According to the datasheet this could be 0x20000. It does not matter
> > > > though, as there's nothing in it past 0x300.
> > >
> > > Good catch.
> > >
> >
> > Are you planning on sending a v6 or you want me to apply this and you
> > can post a patch on top?
>
> Yeah, sorry, I wasn't very clear there. I was hoping just to do a follow-up
> patch with the cleanups if you're okay with that?
That's fine. V5 applied for next, thanks!
Kind regards
Uffe
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Andrew Jeffery @ 2019-08-22 11:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CAPDyKFrDPxFMm710Z25i-euOT2rrgCNXVa4na-fye0xamMXq_A@mail.gmail.com>
On Thu, 22 Aug 2019, at 21:15, Ulf Hansson wrote:
> On Thu, 15 Aug 2019 at 07:37, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> >
> > On Thu, 15 Aug 2019, at 15:06, Joel Stanley wrote:
> > > On Wed, 7 Aug 2019 at 00:38, Andrew Jeffery <andrew@aj.id.au> wrote:
> > > >
> > > > The ASPEED SD/SDIO/MMC controller exposes two slots implementing the
> > > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> > > > data bus if only a single slot is enabled.
> > > >
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > >
> > > Reviewed-by: Joel Stanley <joel@jms.id.au>
> > >
> > > Two minor comments below.
> > >
> > > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > > > @@ -0,0 +1,105 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > >
> > > No "Copyright IBM" ?
> >
> > I'm going rogue.
> >
> > That reminds me I should chase up where we got to with the binding
> > licensing.
> >
> > >
> > > > +%YAML 1.2
> > > > +---
> > >
> > > > +
> > > > +examples:
> > > > + - |
> > > > + #include <dt-bindings/clock/aspeed-clock.h>
> > > > + sdc at 1e740000 {
> > > > + compatible = "aspeed,ast2500-sd-controller";
> > > > + reg = <0x1e740000 0x100>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <1>;
> > > > + ranges = <0 0x1e740000 0x10000>;
> > >
> > > According to the datasheet this could be 0x20000. It does not matter
> > > though, as there's nothing in it past 0x300.
> >
> > Good catch.
> >
>
> Are you planning on sending a v6 or you want me to apply this and you
> can post a patch on top?
Yeah, sorry, I wasn't very clear there. I was hoping just to do a follow-up
patch with the cleanups if you're okay with that?
Andrew
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: mmc: Document Aspeed SD controller
From: Ulf Hansson @ 2019-08-22 11:45 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <6c94aada-9c4a-4f55-9a43-349282ad12af@www.fastmail.com>
On Thu, 15 Aug 2019 at 07:37, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 15 Aug 2019, at 15:06, Joel Stanley wrote:
> > On Wed, 7 Aug 2019 at 00:38, Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > > The ASPEED SD/SDIO/MMC controller exposes two slots implementing the
> > > SDIO Host Specification v2.00, with 1 or 4 bit data buses, or an 8 bit
> > > data bus if only a single slot is enabled.
> > >
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> >
> > Reviewed-by: Joel Stanley <joel@jms.id.au>
> >
> > Two minor comments below.
> >
> > > +++ b/Documentation/devicetree/bindings/mmc/aspeed,sdhci.yaml
> > > @@ -0,0 +1,105 @@
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> >
> > No "Copyright IBM" ?
>
> I'm going rogue.
>
> That reminds me I should chase up where we got to with the binding
> licensing.
>
> >
> > > +%YAML 1.2
> > > +---
> >
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/clock/aspeed-clock.h>
> > > + sdc at 1e740000 {
> > > + compatible = "aspeed,ast2500-sd-controller";
> > > + reg = <0x1e740000 0x100>;
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > + ranges = <0 0x1e740000 0x10000>;
> >
> > According to the datasheet this could be 0x20000. It does not matter
> > though, as there's nothing in it past 0x300.
>
> Good catch.
>
Are you planning on sending a v6 or you want me to apply this and you
can post a patch on top?
Kind regards
Uffe
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Ivan Mikhaylov @ 2019-08-22 9:15 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821163220.GA11547@roeck-us.net>
On Wed, 2019-08-21 at 09:32 -0700, Guenter Roeck wrote:
>
> > + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> > + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> > + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
>
> The variable reflects the _boot status_. It should not change after booting.
Okay, then perhaps may we set 'status' handler for watchdog device and check
'status' file? Right now 'bootstatus' and 'status' are same because there is no
handler for 'status'.
> > +
> > + return size;
> > +}
> > +static DEVICE_ATTR_WO(access_cs0);
> > +
> > +static struct attribute *bswitch_attrs[] = {
> > + &dev_attr_access_cs0.attr,
> > + NULL
> > +};
> > +ATTRIBUTE_GROUPS(bswitch);
> > +
> > static const struct watchdog_ops aspeed_wdt_ops = {
> > .start = aspeed_wdt_start,
> > .stop = aspeed_wdt_stop,
> > @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device
> > *pdev)
> >
> > wdt->ctrl = WDT_CTRL_1MHZ_CLK;
> >
> > + if (of_property_read_bool(np, "aspeed,alt-boot"))
> > + wdt->wdd.groups = bswitch_groups;
> > +
> Why does this have to be separate to the existing evaluation of
> aspeed,alt-boot, and why does the existing code not work ?
>
> Also, is it guaranteed that this does not interfer with existing
> support for alt-boot ?
It doesn't, it just provides for ast2400 switch to cs0 at side 1(cs1). Problem
is that only one flash chip(side 1/cs1) is accessible on alternate boot, there
is citation from the documentation in commit body. So if by some reason side 0
is corrupted, need to switch into alternate boot to cs1, do the load from it,
drop that bit to make side 0 accessible and do the flash of first side. On
ast2500/2600 this problem is solved already, in alternate boot there both flash
chips are present. It's additional requirement for alternate boot on ast2400, to
make the possibility to access at all side 0 flash chip after we boot to the
alternate side.
^ permalink raw reply
* [PATCH 7/7] ARM: configs: aspeed_g5: Enable AST2600
From: Joel Stanley @ 2019-08-22 4:43 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <faf79f9d-baa1-4c8c-a35b-c5d97ad6df8b@www.fastmail.com>
On Thu, 22 Aug 2019 at 02:07, Andrew Jeffery <andrew@aj.id.au> wrote:
> > +CONFIG_MMC=y
> > +CONFIG_MMC_SDHCI=y
> > +CONFIG_MMC_SDHCI_PLTFM=y
> > +CONFIG_MMC_SDHCI_OF_ASPEED=y
>
> The patches haven't yet been applied to the MMC tree, maybe we should
> add this later?
When enabling drivers in the same merge window as they go into the
tree we will always be in this situation.
If the driver doesn't make it in this merge window, or first has has
changes, the worst that will happen is the kconfig name changes and I
need to update it later. I think we're safe to include it as-is.
Thanks for the review.
Cheers,
Joel
^ permalink raw reply
* [PATCH 1/7] dt-bindings: arm: cpus: Add ASPEED SMP
From: Andrew Jeffery @ 2019-08-22 2:08 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-2-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:25, Joel Stanley wrote:
> The AST2600 SoC contains two CPUs and requires the operating system to
> bring the second one out of firmware.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply
* [PATCH 7/7] ARM: configs: aspeed_g5: Enable AST2600
From: Andrew Jeffery @ 2019-08-22 2:07 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-8-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:26, Joel Stanley wrote:
> CONFIG_STRICT_KERNEL_RWX is enabled by default with ARMv7.
>
> Turn on HIGHMEM as the EVB has 2GB of RAM, and not all is usable without
> hihgmem.
>
> The SoC contains Cortex A7 supporting VFP and has two CPUs.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/configs/aspeed_g5_defconfig | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/configs/aspeed_g5_defconfig
> b/arch/arm/configs/aspeed_g5_defconfig
> index 426d8e0c9890..597536cc9573 100644
> --- a/arch/arm/configs/aspeed_g5_defconfig
> +++ b/arch/arm/configs/aspeed_g5_defconfig
> @@ -21,21 +21,26 @@ CONFIG_PERF_EVENTS=y
> CONFIG_SLAB=y
> CONFIG_SLAB_FREELIST_RANDOM=y
> CONFIG_ARCH_MULTI_V6=y
> -# CONFIG_ARCH_MULTI_V7 is not set
> CONFIG_ARCH_ASPEED=y
> CONFIG_MACH_ASPEED_G5=y
> +CONFIG_MACH_ASPEED_G6=y
> # CONFIG_CACHE_L2X0 is not set
> +CONFIG_SMP=y
> +# CONFIG_ARM_CPU_TOPOLOGY is not set
> CONFIG_VMSPLIT_2G=y
> +CONFIG_NR_CPUS=2
> +CONFIG_HIGHMEM=y
> CONFIG_UACCESS_WITH_MEMCPY=y
> CONFIG_SECCOMP=y
> # CONFIG_ATAGS is not set
> CONFIG_ZBOOT_ROM_TEXT=0x0
> CONFIG_ZBOOT_ROM_BSS=0x0
> CONFIG_KEXEC=y
> -# CONFIG_SUSPEND is not set
> +CONFIG_VFP=y
> +CONFIG_NEON=y
> +CONFIG_KERNEL_MODE_NEON=y
> CONFIG_FIRMWARE_MEMMAP=y
> CONFIG_JUMP_LABEL=y
> -CONFIG_STRICT_KERNEL_RWX=y
> # CONFIG_BLK_DEV_BSG is not set
> # CONFIG_BLK_DEBUG_FS is not set
> # CONFIG_MQ_IOSCHED_DEADLINE is not set
> @@ -140,10 +145,12 @@ CONFIG_ASPEED_BT_IPMI_BMC=y
> CONFIG_HW_RANDOM_TIMERIOMEM=y
> # CONFIG_I2C_COMPAT is not set
> CONFIG_I2C_CHARDEV=y
> +CONFIG_I2C_MUX=y
> CONFIG_I2C_MUX_PCA9541=y
> CONFIG_I2C_MUX_PCA954x=y
> CONFIG_I2C_ASPEED=y
> CONFIG_I2C_FSI=y
> +CONFIG_SPI=y
> CONFIG_GPIOLIB=y
> CONFIG_GPIO_SYSFS=y
> CONFIG_GPIO_ASPEED=y
> @@ -194,6 +201,10 @@ CONFIG_USB_CONFIGFS_F_LB_SS=y
> CONFIG_USB_CONFIGFS_F_FS=y
> CONFIG_USB_CONFIGFS_F_HID=y
> CONFIG_USB_CONFIGFS_F_PRINTER=y
> +CONFIG_MMC=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_PLTFM=y
> +CONFIG_MMC_SDHCI_OF_ASPEED=y
The patches haven't yet been applied to the MMC tree, maybe we should
add this later?
Anyway,
Acked-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply
* [PATCH 6/7] ARM: configs: multi_v7: Add ASPEED G6
From: Andrew Jeffery @ 2019-08-22 2:04 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-7-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:26, Joel Stanley wrote:
> This adds the ASPEED AST2600 system and associated ASPEED devices so we
> get build coverage.
>
> The changes to the UART configuration to ensure the default console
> (UART5) works.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Acked-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply
* [PATCH 5/7] ARM: dts: aspeed: Add AST2600 and EVB
From: Andrew Jeffery @ 2019-08-22 1:58 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-6-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:26, Joel Stanley wrote:
> The AST2600 is a new SoC by ASPEED. It contains a dual core Cortex A7
> CPU and shares many periperhals with the existing AST2400 and AST2500.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/aspeed-ast2600-evb.dts | 44 ++++
> arch/arm/boot/dts/aspeed-g6.dtsi | 266 +++++++++++++++++++++++
> 3 files changed, 311 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb.dts
> create mode 100644 arch/arm/boot/dts/aspeed-g6.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 247e556de48e..2d8d29e5686d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1276,6 +1276,7 @@ dtb-$(CONFIG_ARCH_MILBEAUT) +=
> milbeaut-m10v-evb.dtb
> dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
> dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-ast2500-evb.dtb \
> + aspeed-ast2600-evb.dtb \
> aspeed-bmc-arm-centriq2400-rep.dtb \
> aspeed-bmc-arm-stardragon4800-rep2.dtb \
> aspeed-bmc-facebook-cmm.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
> b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
> new file mode 100644
> index 000000000000..7f2528e084b5
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 IBM Corp.
> +
> +/dts-v1/;
> +
> +#include "aspeed-g6.dtsi"
> +
> +/ {
> + model = "AST2600 EVB";
> + compatible = "aspeed,ast2600";
> +
> + aliases {
> + serial4 = &uart5;
> + };
> +
> + chosen {
> + bootargs = "console=ttyS4,115200n8";
> + };
> +
> + memory at 80000000 {
> + device_type = "memory";
> + reg = <0x80000000 0x80000000>;
> + };
> +};
> +
> +&mdio1 {
> + status = "okay";
> +
> + ethphy1: ethernet-phy at 0 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <0>;
> + };
> +};
> +
> +&mac1 {
> + status = "okay";
> +
> + phy-mode = "rgmii";
> + phy-handle = <ðphy1>;
> +};
> +
> +&emmc {
> + status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi
> b/arch/arm/boot/dts/aspeed-g6.dtsi
> new file mode 100644
> index 000000000000..9f9931541060
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-g6.dtsi
> @@ -0,0 +1,266 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright 2019 IBM Corp.
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/ast2600-clock.h>
> +
> +/ {
> + model = "Aspeed BMC";
> + compatible = "aspeed,ast2600";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + interrupt-parent = <&gic>;
> +
> + aliases {
> + serial4 = &uart5;
> + };
> +
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + enable-method = "aspeed,ast2600-smp";
> +
> + cpu at f00 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <0xf00>;
> + };
> +
> + cpu at f01 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <0xf01>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv7-timer";
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) |
> IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
> + clocks = <&syscon ASPEED_CLK_HPLL>;
> + arm,cpu-registers-not-fw-configured;
> + };
> +
> + ahb {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
> + ranges;
> +
> + gic: interrupt-controller at 40461000 {
> + compatible = "arm,cortex-a7-gic";
> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) |
> IRQ_TYPE_LEVEL_HIGH)>;
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + interrupt-parent = <&gic>;
> + reg = <0x40461000 0x1000>,
> + <0x40462000 0x1000>,
> + <0x40464000 0x2000>,
> + <0x40466000 0x2000>;
> + };
> +
> + mdio0: mdio at 1e650000 {
> + compatible = "aspeed,ast2600-mdio";
> + reg = <0x1e650000 0x8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + mdio1: mdio at 1e650008 {
> + compatible = "aspeed,ast2600-mdio";
> + reg = <0x1e650008 0x8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + mdio2: mdio at 1e650010 {
> + compatible = "aspeed,ast2600-mdio";
> + reg = <0x1e650010 0x8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + mdio3: mdio at 1e650018 {
> + compatible = "aspeed,ast2600-mdio";
> + reg = <0x1e650018 0x8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + mac0: ftgmac at 1e660000 {
> + compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";
> + reg = <0x1e660000 0x180>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_MAC1CLK>;
> + status = "disabled";
> + };
> +
> + mac1: ftgmac at 1e680000 {
> + compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";
> + reg = <0x1e680000 0x180>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_MAC2CLK>;
> + status = "disabled";
> + };
> +
> + mac2: ftgmac at 1e670000 {
> + compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";
> + reg = <0x1e670000 0x180>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_MAC3CLK>;
> + status = "disabled";
> + };
> +
> + mac3: ftgmac at 1e690000 {
> + compatible = "aspeed,ast2600-mac", "faraday,ftgmac100";
> + reg = <0x1e690000 0x180>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_MAC4CLK>;
> + status = "disabled";
> + };
> +
> + apb {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + syscon: syscon at 1e6e2000 {
> + compatible = "aspeed,ast2600-scu", "syscon", "simple-mfd";
> + reg = <0x1e6e2000 0x1000>;
> + ranges = <0 0x1e6e2000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> +
> + pinctrl: pinctrl {
> + compatible = "aspeed,ast2600-pinctrl";
> + };
> +
> + smp-memram at 180 {
> + compatible = "aspeed,ast2600-smpmem";
> + reg = <0x180 0x40>;
> + };
> + };
> +
> + rng: hwrng at 1e6e2524 {
> + compatible = "timeriomem_rng";
> + reg = <0x1e6e2524 0x4>;
> + period = <1>;
> + quality = <100>;
> + };
> +
> + rtc: rtc at 1e781000 {
> + compatible = "aspeed,ast2600-rtc";
> + reg = <0x1e781000 0x18>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> + uart5: serial at 1e784000 {
> + compatible = "ns16550a";
> + reg = <0x1e784000 0x1000>;
> + reg-shift = <2>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_GATE_UART5CLK>;
> + no-loopback-test;
> + };
> +
> + wdt1: watchdog at 1e785000 {
> + compatible = "aspeed,ast2600-wdt";
> + reg = <0x1e785000 0x40>;
> + };
> +
> + wdt2: watchdog at 1e785040 {
> + compatible = "aspeed,ast2600-wdt";
> + reg = <0x1e785040 0x40>;
> + status = "disabled";
> + };
> +
> + wdt3: watchdog at 1e785080 {
> + compatible = "aspeed,ast2600-wdt";
> + reg = <0x1e785080 0x40>;
> + status = "disabled";
> + };
> +
> + wdt4: watchdog at 1e7850C0 {
> + compatible = "aspeed,ast2600-wdt";
> + reg = <0x1e7850C0 0x40>;
> + status = "disabled";
> + };
> +
> + sdc: sdc at 1e740000 {
> + compatible = "aspeed,ast2600-sd-controller";
> + reg = <0x1e740000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x1e740000 0x10000>;
> + clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
> + status = "disabled";
> +
> + sdhci0: sdhci at 1e740100 {
> + compatible = "aspeed,ast2600-sdhci", "sdhci";
> + reg = <0x100 0x100>;
> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> + sdhci,auto-cmd12;
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + status = "disabled";
> + };
> +
> + sdhci1: sdhci at 1e740200 {
> + compatible = "aspeed,ast2600-sdhci", "sdhci";
> + reg = <0x200 0x100>;
> + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
> + sdhci,auto-cmd12;
> + clocks = <&syscon ASPEED_CLK_SDIO>;
> + status = "disabled";
> + };
> + };
> +
> + emmc: sdc at 1e750000 {
> + compatible = "aspeed,ast2600-sd-controller";
> + reg = <0x1e750000 0x100>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x1e750000 0x10000>;
> + clocks = <&syscon ASPEED_CLK_GATE_EMMCCLK>;
> + status = "disabled";
> +
> + sdhci at 1e750100 {
> + compatible = "aspeed,ast2600-sdhci";
> + reg = <0x100 0x100>;
> + sdhci,auto-cmd12;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&syscon ASPEED_CLK_EMMC>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_emmc_default>;
> + };
> + };
> + };
> + };
> +};
> +
> +&pinctrl {
> + pinctrl_emmc_default: emmc_default {
> + function = "SD3";
> + groups = "SD3";
> + };
I need to send some fixes for pinmux along with the dt patche, but this
will do for the moment.
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
^ permalink raw reply
* [PATCH 3/7] ARM: aspeed: Add ASPEED AST2600 architecture
From: Andrew Jeffery @ 2019-08-22 1:28 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-4-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:26, Joel Stanley wrote:
> The AST2600 is a Cortex A7 dual core CPU that uses the ARM GIC for
> interrupts and ARM timer as a clocksource.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> arch/arm/mach-aspeed/Kconfig | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig
> index 2979aa4daeea..56007b0b6120 100644
> --- a/arch/arm/mach-aspeed/Kconfig
> +++ b/arch/arm/mach-aspeed/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> menuconfig ARCH_ASPEED
> bool "Aspeed BMC architectures"
> - depends on ARCH_MULTI_V5 || ARCH_MULTI_V6
> + depends on ARCH_MULTI_V5 || ARCH_MULTI_V6 || ARCH_MULTI_V7
> select SRAM
> select WATCHDOG
> select ASPEED_WATCHDOG
> @@ -33,4 +33,16 @@ config MACH_ASPEED_G5
> Say yes if you intend to run on an Aspeed ast2500 or similar
> fifth generation Aspeed BMCs.
>
> +config MACH_ASPEED_G6
> + bool "Aspeed SoC 6th Generation"
> + depends on ARCH_MULTI_V7
> + select CPU_V7
> + select PINCTRL_ASPEED_G6
> + select ARM_GIC
> + select HAVE_ARM_ARCH_TIMER
> + select HAVE_SMP
> + help
> + Say yes if you intend to run on an Aspeed ast2600 or similar
> + sixth generation Aspeed BMCs.
> +
> endif
> --
> 2.23.0.rc1
>
>
^ permalink raw reply
* [PATCH 2/7] ARM: aspeed: Select timer in each SoC
From: Andrew Jeffery @ 2019-08-22 1:27 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-3-joel@jms.id.au>
On Wed, 21 Aug 2019, at 15:26, Joel Stanley wrote:
> In preparation for adding the ast2600 which does not use this timer.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> arch/arm/mach-aspeed/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-aspeed/Kconfig b/arch/arm/mach-aspeed/Kconfig
> index a15c3a291386..2979aa4daeea 100644
> --- a/arch/arm/mach-aspeed/Kconfig
> +++ b/arch/arm/mach-aspeed/Kconfig
> @@ -5,7 +5,6 @@ menuconfig ARCH_ASPEED
> select SRAM
> select WATCHDOG
> select ASPEED_WATCHDOG
> - select FTTMR010_TIMER
> select MFD_SYSCON
> select PINCTRL
> help
> @@ -18,6 +17,7 @@ config MACH_ASPEED_G4
> depends on ARCH_MULTI_V5
> select CPU_ARM926T
> select PINCTRL_ASPEED_G4
> + select FTTMR010_TIMER
> help
> Say yes if you intend to run on an Aspeed ast2400 or similar
> fourth generation BMCs, such as those used by OpenPower Power8
> @@ -28,6 +28,7 @@ config MACH_ASPEED_G5
> depends on ARCH_MULTI_V6
> select CPU_V6
> select PINCTRL_ASPEED_G5
> + select FTTMR010_TIMER
> help
> Say yes if you intend to run on an Aspeed ast2500 or similar
> fifth generation Aspeed BMCs.
> --
> 2.23.0.rc1
>
>
^ permalink raw reply
* [PATCH v3 00/11] Symbol Namespaces
From: Nicolas Pitre @ 2019-08-21 20:48 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821133737.GB4890@kroah.com>
On Wed, 21 Aug 2019, Greg KH wrote:
> On Wed, Aug 21, 2019 at 08:46:47AM -0400, Nicolas Pitre wrote:
>
> > One solution for drastically reducing the effective export surface is to
> > have CONFIG_TRIM_UNUSED_KSYMS=y. This is more extreme than symbol
> > namespace, but might be worth mentioning nevertheless.
>
> Oh that's amazing, I never noticed that feature. That is a nice thing,
> thanks for pointing it out.
For those interested, this feature was demonstrated with numbers here:
https://lwn.net/Articles/746780/
Nicolas
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-21 18:10 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <9e7fe5cc-ba1b-b8b6-69c5-c3c6cf508a36@yadro.com>
On Wed, Aug 21, 2019 at 08:42:24PM +0300, Alexander Amelkin wrote:
> 21.08.2019 19:32, Guenter Roeck wrote:
> > On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
> >> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> >> to clear out boot code source and re-enable access to the primary SPI flash
> >> chip while booted via wdt2 from the alternate chip.
> >>
> >> AST2400 datasheet says:
> >> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> >> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> >> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> >> register WDT30.bit[1]."
> > Is there reason to not do this automatically when loading the module
> > in alt-boot mode ? What means does userspace have to determine if CS0
> > or CS1 is active at any given time ? If there is reason to ever have CS1
> > active instead of CS0, what means would userspace have to enable it ?
>
> Yes, there is. The driver is loaded long before the filesystems are mounted. The filesystems, in the event of alternate/recovery boot, need to be mounted from the same chip that the kernel was booted. For one reason because the main chip at CS0 is most probably corrupt. If you clear that bit when driver is loaded, your software will not know that and will try to mount the wrong filesystems. The whole idea of ASPEED's switching chipselects is to have identical firmware in both chips, without the need to process the alternate boot state in any way except for indicating a successful boot and restoring access to CS0 when needed.
>
> The userspace can read bootstatus sysfs node to determine if an alternate boot has occured.
>
> With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot from the primary flash chip (at CS0) and disable the watchdog to indicate a successful boot. When that happens, both CS0 and CS1 controls? get routed in hardware to CS1 line, making the primary flash chip inaccessible. Depending on the architecture of the user-space software, it may choose to re-enable access to the primary chip via CS0 at different times. There must be a way to do so.
>
So by activating cs0, userspace would essentially pull its own root file system
from underneath itself ?
> > If userspace can not really determine if CS1 or CS0 is active, all it could
> > ever do was to enable CS0 to be in a deterministic state. If so, it doesn't
> > make sense to ever have CS1 active, and re-enabling CS0 could be automatic.
> >
> > Similar, if CS1 can ever be enabled, there is no means for userspace to ensure
> > that some other application did not re-enable CS0 while it believes that CS1
> > is enabled. If there is no means for userspace to enable CS1, it can never be
> > sure what is enabled (because some other entity may have enabled CS0 while
> > userspace just thought that CS1 is still enabled). Again, the only means
> > to guarantee a well defined state would be to explicitly enable CS0 and provive
> > no means to enable CS1. Again, this could be done during boot, not requiring
> > an explicit request from userspace.
>
> Please understand that activation of CS1 in place of CS0 is NOT a software choice!
>
>
> >> + if (unlikely(!wdt))
> >> + return -ENODEV;
> >> +
> > How would this ever happen, and how / where is drvdata set to NULL ?
>
> This is purely for robustness. Seeing a pointer obtained via a function accessed without first checking it for validity makes me nervous.
>
This is not how kernel code is commonly written.
Sure, we could add similar checks to each sysfs access code in the kernel,
blowing up its size significantly. I do not see a point of this.
> This code most probably adds nothing at the assembly level.
>
That seems quite unlikely. Please demonstrate.
> >
> >> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> >> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> >> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> > The variable reflects the _boot status_. It should not change after booting.
> Is there any documentation that dictates that? All I could find is
>
> "bootstatus: status of the device after booting". That doesn't look to me like it absolutely can not change to reflect the updated status (that is, to reflect that the originally set up alternate CS routing has been reset to normal).
>
You choose to interpret "after booting" in a kind of novel way,
which I find a bit disturbing. I am not really sure how else to
describe "boot status" in a way that does not permit such
reinterpratation of the term.
On top of that, how specifically would "WDIOF_EXTERN1" reflect
what you claim it does ? Not only you are hijacking bootstatus9
(which is supposed to describe the reason for a reboot), you
are also hijacking WDIOF_EXTERN1. That seems highly arbitrary
to me, and is not really how an API/ABI should be used.
Guenter
> If you absolutely disallow that, I think we could make 'access_cs0' readable instead, so it could report the current state of the boot code selection bit. Reverted, I suppose. That way 'access_cs0' would report 1 after 1 has been written to it (it wouldn't be possible to write a zero).
>
> > @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> >
> > wdt->ctrl = WDT_CTRL_1MHZ_CLK;
> >
> > + if (of_property_read_bool(np, "aspeed,alt-boot"))
> > + wdt->wdd.groups = bswitch_groups;
> > +
> > Why does this have to be separate to the existing evaluation of
> > aspeed,alt-boot, and why does the existing code not work ?
> >
> > Also, is it guaranteed that this does not interfer with existing
> > support for alt-boot ?
>
> I think Ivan will comment on this.
>
> With best regards,
> Alexander Amelkin,
> BIOS/BMC Team Lead, YADRO
> https://yadro.com
>
>
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Alexander Amelkin @ 2019-08-21 17:42 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821163220.GA11547@roeck-us.net>
21.08.2019 19:32, Guenter Roeck wrote:
> On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
>> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
>> to clear out boot code source and re-enable access to the primary SPI flash
>> chip while booted via wdt2 from the alternate chip.
>>
>> AST2400 datasheet says:
>> "In the 2nd flash booting mode, all the address mapping to CS0# would be
>> re-directed to CS1#. And CS0# is not accessable under this mode. To access
>> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
>> register WDT30.bit[1]."
> Is there reason to not do this automatically when loading the module
> in alt-boot mode ? What means does userspace have to determine if CS0
> or CS1 is active at any given time ? If there is reason to ever have CS1
> active instead of CS0, what means would userspace have to enable it ?
Yes, there is. The driver is loaded long before the filesystems are mounted. The filesystems, in the event of alternate/recovery boot, need to be mounted from the same chip that the kernel was booted. For one reason because the main chip at CS0 is most probably corrupt. If you clear that bit when driver is loaded, your software will not know that and will try to mount the wrong filesystems. The whole idea of ASPEED's switching chipselects is to have identical firmware in both chips, without the need to process the alternate boot state in any way except for indicating a successful boot and restoring access to CS0 when needed.
The userspace can read bootstatus sysfs node to determine if an alternate boot has occured.
With ASPEED, CS1 is activated automatically by wdt2 when system fails to boot from the primary flash chip (at CS0) and disable the watchdog to indicate a successful boot. When that happens, both CS0 and CS1 controls? get routed in hardware to CS1 line, making the primary flash chip inaccessible. Depending on the architecture of the user-space software, it may choose to re-enable access to the primary chip via CS0 at different times. There must be a way to do so.
> If userspace can not really determine if CS1 or CS0 is active, all it could
> ever do was to enable CS0 to be in a deterministic state. If so, it doesn't
> make sense to ever have CS1 active, and re-enabling CS0 could be automatic.
>
> Similar, if CS1 can ever be enabled, there is no means for userspace to ensure
> that some other application did not re-enable CS0 while it believes that CS1
> is enabled. If there is no means for userspace to enable CS1, it can never be
> sure what is enabled (because some other entity may have enabled CS0 while
> userspace just thought that CS1 is still enabled). Again, the only means
> to guarantee a well defined state would be to explicitly enable CS0 and provive
> no means to enable CS1. Again, this could be done during boot, not requiring
> an explicit request from userspace.
Please understand that activation of CS1 in place of CS0 is NOT a software choice!
>> + if (unlikely(!wdt))
>> + return -ENODEV;
>> +
> How would this ever happen, and how / where is drvdata set to NULL ?
This is purely for robustness. Seeing a pointer obtained via a function accessed without first checking it for validity makes me nervous.
This code most probably adds nothing at the assembly level.
>
>> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
>> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
>> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> The variable reflects the _boot status_. It should not change after booting.
Is there any documentation that dictates that? All I could find is
"bootstatus: status of the device after booting". That doesn't look to me like it absolutely can not change to reflect the updated status (that is, to reflect that the originally set up alternate CS routing has been reset to normal).
If you absolutely disallow that, I think we could make 'access_cs0' readable instead, so it could report the current state of the boot code selection bit. Reverted, I suppose. That way 'access_cs0' would report 1 after 1 has been written to it (it wouldn't be possible to write a zero).
> @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>
> wdt->ctrl = WDT_CTRL_1MHZ_CLK;
>
> + if (of_property_read_bool(np, "aspeed,alt-boot"))
> + wdt->wdd.groups = bswitch_groups;
> +
> Why does this have to be separate to the existing evaluation of
> aspeed,alt-boot, and why does the existing code not work ?
>
> Also, is it guaranteed that this does not interfer with existing
> support for alt-boot ?
I think Ivan will comment on this.
With best regards,
Alexander Amelkin,
BIOS/BMC Team Lead, YADRO
https://yadro.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20190821/a5922211/attachment-0001.sig>
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Guenter Roeck @ 2019-08-21 16:32 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1f2cd155057e5ab0cdb20a9a11614bbb09bb49ad.camel@yadro.com>
On Wed, Aug 21, 2019 at 06:57:43PM +0300, Ivan Mikhaylov wrote:
> Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
> to clear out boot code source and re-enable access to the primary SPI flash
> chip while booted via wdt2 from the alternate chip.
>
> AST2400 datasheet says:
> "In the 2nd flash booting mode, all the address mapping to CS0# would be
> re-directed to CS1#. And CS0# is not accessable under this mode. To access
> CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
> register WDT30.bit[1]."
Is there reason to not do this automatically when loading the module
in alt-boot mode ? What means does userspace have to determine if CS0
or CS1 is active at any given time ? If there is reason to ever have CS1
active instead of CS0, what means would userspace have to enable it ?
If userspace can not really determine if CS1 or CS0 is active, all it could
ever do was to enable CS0 to be in a deterministic state. If so, it doesn't
make sense to ever have CS1 active, and re-enabling CS0 could be automatic.
Similar, if CS1 can ever be enabled, there is no means for userspace to ensure
that some other application did not re-enable CS0 while it believes that CS1
is enabled. If there is no means for userspace to enable CS1, it can never be
sure what is enabled (because some other entity may have enabled CS0 while
userspace just thought that CS1 is still enabled). Again, the only means
to guarantee a well defined state would be to explicitly enable CS0 and provive
no means to enable CS1. Again, this could be done during boot, not requiring
an explicit request from userspace.
>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
> drivers/watchdog/aspeed_wdt.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index cc71861e033a..858e62f1c7ba 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -53,6 +53,8 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
> #define WDT_CTRL_ENABLE BIT(0)
> #define WDT_TIMEOUT_STATUS 0x10
> #define WDT_TIMEOUT_STATUS_BOOT_SECONDARY BIT(1)
> +#define WDT_CLEAR_TIMEOUT_STATUS 0x14
> +#define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0)
>
> /*
> * WDT_RESET_WIDTH controls the characteristics of the external pulse (if
> @@ -165,6 +167,29 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
> return 0;
> }
>
> +static ssize_t access_cs0_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct aspeed_wdt *wdt = dev_get_drvdata(dev);
> +
> + if (unlikely(!wdt))
> + return -ENODEV;
> +
How would this ever happen, and how / where is drvdata set to NULL ?
> + writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> + wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> + wdt->wdd.bootstatus |= WDIOF_EXTERN1;
The variable reflects the _boot status_. It should not change after booting.
> +
> + return size;
> +}
> +static DEVICE_ATTR_WO(access_cs0);
> +
> +static struct attribute *bswitch_attrs[] = {
> + &dev_attr_access_cs0.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(bswitch);
> +
> static const struct watchdog_ops aspeed_wdt_ops = {
> .start = aspeed_wdt_start,
> .stop = aspeed_wdt_stop,
> @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>
> wdt->ctrl = WDT_CTRL_1MHZ_CLK;
>
> + if (of_property_read_bool(np, "aspeed,alt-boot"))
> + wdt->wdd.groups = bswitch_groups;
> +
Why does this have to be separate to the existing evaluation of
aspeed,alt-boot, and why does the existing code not work ?
Also, is it guaranteed that this does not interfer with existing
support for alt-boot ?
> /*
> * Control reset on a per-device basis to ensure the
> * host is not affected by a BMC reboot
> @@ -309,6 +337,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
> wdt->wdd.bootstatus = WDIOF_CARDRESET;
>
> + dev_set_drvdata(dev, wdt);
> +
> return devm_watchdog_register_device(dev, &wdt->wdd);
> }
>
> --
> 2.20.1
>
>
^ permalink raw reply
* [PATCH v3 11/11] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
From: Matthias Maennich @ 2019-08-21 16:28 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821145911.GA6521@roeck-us.net>
Hi Guenter!
On Wed, 21 Aug, 07:59, Guenter Roeck wrote:
>On Wed, Aug 21, 2019 at 12:49:26PM +0100, Matthias Maennich wrote:
>> Modules using these symbols are required to explicitly import the
>> namespace. This patch was generated with the following steps and serves
>> as a reference to use the symbol namespace feature:
>>
>> 1) Use EXPORT_SYMBOL_NS* macros instead of EXPORT_SYMBOL* for symbols
>> in watchdog_core.c
>> 2) make (see warnings during modpost about missing imports)
>> 3) make nsdeps
>>
>> I used 'allmodconfig' for the above steps to ensure all occurrences are
>> patched.
>>
>> Defining DEFAULT_SYMBOL_NAMESPACE in the Makefile is not trivial in this
>> case as not only watchdog_core is defined in drivers/watchdog/Makefile.
>> Hence this patch uses the variant of using the EXPORT_SYMBOL_NS* macros
>> to export into a different namespace.
>>
>I don't have the context, and thus I am missing the point of this patch
>set. Whatever it is supposed to accomplish, it seems extreme to me
>to require extra code in each driver for it.
>
Unfortunately, get_maintainer.pl has helped me too much and this series
got blocked by some mailing lists due to the large amount of recipients.
Following versions will be sent to the previous audience + the
linux-watchdog list.
For context, the full series (including previous versions) can be found
on lore at
https://lore.kernel.org/lkml/20180716122125.175792-1-maco at android.com/
and the cover letter for v3 has made it to linux-amlogic
https://lore.kernel.org/linux-amlogic/20190821114955.12788-1-maennich at google.com/
>Anyway, WATCHDOG_CORE would be the default namespace (if it is what
>I think it is) for watchdog drivers, even though not all watchdog drivers
>use it. As such, I am missing an explanation why defining it in Makefile
>is not trivial. "... as not only watchdog_core is defined in
>drivers/watchdog/Makefile" does not mean anything to me and is not a real
True, that is a bit out of context. Especially considering you did not
receive any other messages of that series.
Defining a namespace a symbol should be exported to can be done in
different ways. All of them effectively change the EXPORT_SYMBOL*
macro's behaviour. The method I am referring to is using
ccflags-y += -DDEFAULT_SYMBOL_NAMESPACE=WATCHDOG_CORE
directly in drivers/watchdog/Makefile. Since this would also apply the
namespace to exports in non-core modules it would be incorrect. Thus I
used the method of applying the namespace directly by changing the
EXPORT_SYMBOL macro expansion.
>explanation. Also, it is not immediately obvious to me why "select
>WATCHDOG_CORE" in Kconfig would not automatically imply that WATCHDOG_CORE
>is used by a given driver, and why it is impossible to use that
>information to avoid the per-driver changes.
>
One intention of this patch series is to make exporting and using of
namespaces explicit. As such, the subsystem exporting symbols is
defining the namespace it exports to and the module using a namespace is
supposed to explicitly declare its usage via import. In case of watchdog
(and probably other cases) it might make sense to find a way to
implicitly import the namespace for in-tree drivers in the same area.
>I am also missing an explanation why WATCHDOG_CORE is going to be a
>separate namespace to start with. Maybe that discussion has happened,
>but I don't recall being advised or asked or told about it. Are we also
>going to have a new HWMON_CORE namespace ? And the same for each other
>subsystem in the kernel ?
>
This very patch is an RFC to demonstrate how Symbol Namespaces would be
used based on the current implementation (the other RFC as part of this
series is for the introduction of the namespace USB_STORAGE).
WATCHDOG_CORE serves as one of two examples. I do not think the two RFC
patches should be merged along with this series.
>Since this is being added to the watchdog API, it will have to be
>documented accordingly. Watchdog driver writers, both inside and outside
>the watchdog subsystem, will need to know that they now have to add an
>additional boilerplate declaration into their drivers.
>
Completely agree. This is just an RFC that omits these details as it
purely focuses on the introduction and consequences of such a namespace
to demonstrate how the feature works.
>Last but not least, combining patches affecting multiple subsystems in a
>single patch will make it difficult to apply and will likely result in
>conflicts. Personally I would prefer a split into one patch per affected
>subsystem. Also, please keep in mind that new pending watchdog drivers
>won't have the new boilerplate.
I understand the point. Especially as I am already now affected by the
long list of recipients when sending this patch. The problem with single
patches here is, that once a symbol is exported into a namespace, all
modules using it have to declare that import to avoid a warning at
compile time and module load time. Hence the all-in-one approach.
Luckily, the patch series also provides a way to address such a warning
(via `make nsdeps`) that creates the necessary source code fix as a
single line per module and namespace right after MODULE_LICENSE(). That
is how this patch was created in the first place.
Cheers,
Matthias
^ permalink raw reply
* [PATCH 1/7] dt-bindings: arm: cpus: Add ASPEED SMP
From: Rob Herring @ 2019-08-21 16:23 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190821055530.8720-2-joel@jms.id.au>
On Wed, Aug 21, 2019 at 12:55 AM Joel Stanley <joel@jms.id.au> wrote:
>
> The AST2600 SoC contains two CPUs and requires the operating system to
> bring the second one out of firmware.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> Documentation/devicetree/bindings/arm/cpus.yaml | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 3/3] watchdog/aspeed: add support for dual boot
From: Ivan Mikhaylov @ 2019-08-21 15:57 UTC (permalink / raw)
To: linux-aspeed
Set WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION into WDT_CLEAR_TIMEOUT_STATUS
to clear out boot code source and re-enable access to the primary SPI flash
chip while booted via wdt2 from the alternate chip.
AST2400 datasheet says:
"In the 2nd flash booting mode, all the address mapping to CS0# would be
re-directed to CS1#. And CS0# is not accessable under this mode. To access
CS0#, firmware should clear the 2nd boot mode register in the WDT2 status
register WDT30.bit[1]."
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
drivers/watchdog/aspeed_wdt.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
index cc71861e033a..858e62f1c7ba 100644
--- a/drivers/watchdog/aspeed_wdt.c
+++ b/drivers/watchdog/aspeed_wdt.c
@@ -53,6 +53,8 @@ MODULE_DEVICE_TABLE(of, aspeed_wdt_of_table);
#define WDT_CTRL_ENABLE BIT(0)
#define WDT_TIMEOUT_STATUS 0x10
#define WDT_TIMEOUT_STATUS_BOOT_SECONDARY BIT(1)
+#define WDT_CLEAR_TIMEOUT_STATUS 0x14
+#define WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION BIT(0)
/*
* WDT_RESET_WIDTH controls the characteristics of the external pulse (if
@@ -165,6 +167,29 @@ static int aspeed_wdt_restart(struct watchdog_device *wdd,
return 0;
}
+static ssize_t access_cs0_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct aspeed_wdt *wdt = dev_get_drvdata(dev);
+
+ if (unlikely(!wdt))
+ return -ENODEV;
+
+ writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
+ wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
+ wdt->wdd.bootstatus |= WDIOF_EXTERN1;
+
+ return size;
+}
+static DEVICE_ATTR_WO(access_cs0);
+
+static struct attribute *bswitch_attrs[] = {
+ &dev_attr_access_cs0.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(bswitch);
+
static const struct watchdog_ops aspeed_wdt_ops = {
.start = aspeed_wdt_start,
.stop = aspeed_wdt_stop,
@@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
wdt->ctrl = WDT_CTRL_1MHZ_CLK;
+ if (of_property_read_bool(np, "aspeed,alt-boot"))
+ wdt->wdd.groups = bswitch_groups;
+
/*
* Control reset on a per-device basis to ensure the
* host is not affected by a BMC reboot
@@ -309,6 +337,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
if (status & WDT_TIMEOUT_STATUS_BOOT_SECONDARY)
wdt->wdd.bootstatus = WDIOF_CARDRESET;
+ dev_set_drvdata(dev, wdt);
+
return devm_watchdog_register_device(dev, &wdt->wdd);
}
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] vesnin: add secondary SPI flash chip
From: Ivan Mikhaylov @ 2019-08-21 15:56 UTC (permalink / raw)
To: linux-aspeed
Adds secondary SPI flash chip into dts for vesnin.
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
index 2ee26c86a32e..db4cc3df61ce 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -81,6 +81,14 @@
label = "bmc";
#include "openbmc-flash-layout.dtsi"
};
+
+ flash at 1 {
+ status = "okay";
+ reg = < 1 >;
+ compatible = "jedec,spi-nor";
+ m25p,fast-read;
+ label = "alt";
+ };
};
&spi {
--
2.20.1
^ permalink raw reply related
* [PATCH 1/3] vesnin: add wdt2 section with alt-boot option
From: Ivan Mikhaylov @ 2019-08-21 15:54 UTC (permalink / raw)
To: linux-aspeed
Adds wdt2 section with 'alt-boot' option into dts for vesnin.
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
index 0b9e29c3212e..2ee26c86a32e 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -222,3 +222,7 @@
&vuart {
status = "okay";
};
+
+&wdt2 {
+ aspeed,alt-boot;
+};
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] add dual-boot support
From: Ivan Mikhaylov @ 2019-08-21 15:52 UTC (permalink / raw)
To: linux-aspeed
ASPEED SoCs support dual-boot feature for SPI Flash.
When strapped appropriately, the SoC starts wdt2 (/dev/watchdog1)
and if within a minute it is not disabled, it goes off and reboots
the SoC from an alternate SPI Flash chip by changing CS0 controls
to actually drive CS1 line.
When booted from alternate chip, in order to access the main chip
at CS0, the user must reset the appropriate bit in the watchdog
hardware. There is no interface that would allow to do that from
an embedded firmware startup script.
This commit implements support for that feature:
* Enable 'alt-boot' option for wdt2
* Enable secondary SPI flash chip
* Make it possible to get access to the primary SPI flash chip at CS0
after booting from the alternate chip at CS1. A sysfs interface is added
to provide an easy way for embedded firmware startup scripts to clear
the chip select bit to gain access to the primary flash chip in order
to allow for recovery of its contents.
Ivan Mikhaylov (3):
vesnin: add wdt2 section with alt-boot option
vesnin: add secondary SPI flash chip
watchdog/aspeed: add support for dual boot
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 12 +++++++++
drivers/watchdog/aspeed_wdt.c | 30 +++++++++++++++++++++
2 files changed, 42 insertions(+)
--
2.20.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox