Support RTC for AST2700

[PATCH v1 7/8] hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and AST2500 EVBs

Posted by Jamin Lin via 3 weeks, 4 days ago

The Write Protect pin of SDHCI model is default active low to match the SDHCI
spec. So, write enable the bit 19 should be 1 and write protected the bit 19
should be 0 at the Present State Register (0x24).

According to the design of AST2500 and AST2600 EVBs, the Write Protected pin
is active high by default. To support it, introduces a new sdhci_wp_invert
property in ASPEED MACHINE state and set it true for AST2500 and AST2600 EVBs
and set "wp_invert" property true of sdhci-generic model.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/arm/aspeed.c         | 8 ++++++++
 include/hw/arm/aspeed.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index b4b1ce9efb..0468602d95 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -403,6 +403,12 @@ static void aspeed_machine_init(MachineState *machine)
                              OBJECT(get_system_memory()), &error_abort);
     object_property_set_link(OBJECT(bmc->soc), "dram",
                              OBJECT(machine->ram), &error_abort);
+    if (amc->sdhci_wp_invert) {
+        for (i = 0; i < bmc->soc->sdhci.num_slots; i++) {
+            object_property_set_bool(OBJECT(&bmc->soc->sdhci.slots[i]),
+                                     "wp-invert", true, &error_abort);
+        }
+    }
     if (machine->kernel_filename) {
         /*
          * When booting with a -kernel command line there is no u-boot
@@ -1308,6 +1314,7 @@ static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
     amc->fmc_model = "mx25l25635e";
     amc->spi_model = "mx25l25635f";
     amc->num_cs    = 1;
+    amc->sdhci_wp_invert = true;
     amc->i2c_init  = ast2500_evb_i2c_init;
     mc->default_ram_size       = 512 * MiB;
     aspeed_machine_class_init_cpus_defaults(mc);
@@ -1409,6 +1416,7 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
     amc->num_cs    = 1;
     amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON | ASPEED_MAC2_ON |
                      ASPEED_MAC3_ON;
+    amc->sdhci_wp_invert = true;
     amc->i2c_init  = ast2600_evb_i2c_init;
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index cbeacb214c..879bdb96ee 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -39,6 +39,7 @@ struct AspeedMachineClass {
     uint32_t macs_mask;
     void (*i2c_init)(AspeedMachineState *bmc);
     uint32_t uart_default;
+    bool sdhci_wp_invert;
 };
 
 
-- 
2.34.1

Re: [PATCH v1 7/8] hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and AST2500 EVBs

Posted by Andrew Jeffery 3 weeks, 3 days ago

On Tue, 2024-10-29 at 17:17 +0800, Jamin Lin wrote:
> The Write Protect pin of SDHCI model is default active low to match
> the SDHCI
> spec. So, write enable the bit 19 should be 1 and write protected the
> bit 19
> should be 0 at the Present State Register (0x24).
> 
> According to the design of AST2500 and AST2600 EVBs, the Write
> Protected pin
> is active high by default. To support it, introduces a new
> sdhci_wp_invert
> property in ASPEED MACHINE state and set it true for AST2500 and
> AST2600 EVBs
> and set "wp_invert" property true of sdhci-generic model.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  hw/arm/aspeed.c         | 8 ++++++++
>  include/hw/arm/aspeed.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index b4b1ce9efb..0468602d95 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -403,6 +403,12 @@ static void aspeed_machine_init(MachineState
> *machine)
>                               OBJECT(get_system_memory()),
> &error_abort);
>      object_property_set_link(OBJECT(bmc->soc), "dram",
>                               OBJECT(machine->ram), &error_abort);
> +    if (amc->sdhci_wp_invert) {
> +        for (i = 0; i < bmc->soc->sdhci.num_slots; i++) {
> +            object_property_set_bool(OBJECT(&bmc->soc-
> >sdhci.slots[i]),
> +                                     "wp-invert", true,
> &error_abort);
> +        }
> +    }
>      if (machine->kernel_filename) {
>          /*
>           * When booting with a -kernel command line there is no u-
> boot
> @@ -1308,6 +1314,7 @@ static void
> aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
>      amc->fmc_model = "mx25l25635e";
>      amc->spi_model = "mx25l25635f";
>      amc->num_cs    = 1;
> +    amc->sdhci_wp_invert = true;
>      amc->i2c_init  = ast2500_evb_i2c_init;
>      mc->default_ram_size       = 512 * MiB;
>      aspeed_machine_class_init_cpus_defaults(mc);
> @@ -1409,6 +1416,7 @@ static void
> aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
>      amc->num_cs    = 1;
>      amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON |
> ASPEED_MAC2_ON |
>                       ASPEED_MAC3_ON;
> +    amc->sdhci_wp_invert = true;
>      amc->i2c_init  = ast2600_evb_i2c_init;
>      mc->default_ram_size = 1 * GiB;
>      aspeed_machine_class_init_cpus_defaults(mc);
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index cbeacb214c..879bdb96ee 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -39,6 +39,7 @@ struct AspeedMachineClass {
>      uint32_t macs_mask;
>      void (*i2c_init)(AspeedMachineState *bmc);
>      uint32_t uart_default;
> +    bool sdhci_wp_invert;

Other than also calling this `sdhci_wp_inverted` to match my comment on
the earlier patch about the model property and devicetree bindings,

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>

RE: [PATCH v1 7/8] hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and AST2500 EVBs

Posted by Jamin Lin 3 weeks, 3 days ago

Hi Andrew,

> Subject: Re: [PATCH v1 7/8] hw/arm/aspeed: Invert sdhci write protected pin
> for AST2600 and AST2500 EVBs
> 
> On Tue, 2024-10-29 at 17:17 +0800, Jamin Lin wrote:
> > The Write Protect pin of SDHCI model is default active low to match
> > the SDHCI spec. So, write enable the bit 19 should be 1 and write
> > protected the bit 19 should be 0 at the Present State Register (0x24).
> >
> > According to the design of AST2500 and AST2600 EVBs, the Write
> > Protected pin is active high by default. To support it, introduces a
> > new sdhci_wp_invert property in ASPEED MACHINE state and set it true
> > for AST2500 and
> > AST2600 EVBs
> > and set "wp_invert" property true of sdhci-generic model.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >  hw/arm/aspeed.c         | 8 ++++++++
> >  include/hw/arm/aspeed.h | 1 +
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > b4b1ce9efb..0468602d95 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -403,6 +403,12 @@ static void aspeed_machine_init(MachineState
> > *machine)
> >                               OBJECT(get_system_memory(
> )),
> > &error_abort);
> >      object_property_set_link(OBJECT(bmc->soc), "dram",
> >                               OBJECT(machine->ram),
> &error_abort);
> > +    if (amc->sdhci_wp_invert) {
> > +        for (i = 0; i < bmc->soc->sdhci.num_slots; i++) {
> > +            object_property_set_bool(OBJECT(&bmc->soc-
> > >sdhci.slots[i]),
> > +                                     "wp-invert", true,
> > &error_abort);
> > +        }
> > +    }
> >      if (machine->kernel_filename) {
> >          /*
> >           * When booting with a -kernel command line there is no u-
> > boot @@ -1308,6 +1314,7 @@ static void
> > aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
> >      amc->fmc_model = "mx25l25635e";
> >      amc->spi_model = "mx25l25635f";
> >      amc->num_cs    = 1;
> > +    amc->sdhci_wp_invert = true;
> >      amc->i2c_init  = ast2500_evb_i2c_init;
> >      mc->default_ram_size       = 512 * MiB;
> >      aspeed_machine_class_init_cpus_defaults(mc);
> > @@ -1409,6 +1416,7 @@ static void
> > aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
> >      amc->num_cs    = 1;
> >      amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON |
> ASPEED_MAC2_ON
> > |
> >                       ASPEED_MAC3_ON;
> > +    amc->sdhci_wp_invert = true;
> >      amc->i2c_init  = ast2600_evb_i2c_init;
> >      mc->default_ram_size = 1 * GiB;
> >      aspeed_machine_class_init_cpus_defaults(mc);
> > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index
> > cbeacb214c..879bdb96ee 100644
> > --- a/include/hw/arm/aspeed.h
> > +++ b/include/hw/arm/aspeed.h
> > @@ -39,6 +39,7 @@ struct AspeedMachineClass {
> >      uint32_t macs_mask;
> >      void (*i2c_init)(AspeedMachineState *bmc);
> >      uint32_t uart_default;
> > +    bool sdhci_wp_invert;
> 
> Other than also calling this `sdhci_wp_inverted` to match my comment on the
> earlier patch about the model property and devicetree bindings,
> 
Thanks for review and suggestion.
Will update it to "sdhci_wp_inverted"

Jamin
> Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>

[PATCH v1 1/8] aspeed/soc: Support RTC for AST2700
[PATCH v1 2/8] hw/timer/aspeed: Fix coding style
[PATCH v1 3/8] hw/timer/aspeed: Fix interrupt status does not be cleared for AST2600
[PATCH v1 4/8] hw/sd/sdhci: Fix coding style
[PATCH v1 5/8] hw/sd/sdhci: Introduce a new Write Protected pin inverted property
[PATCH v1 6/8] hw/sd/aspeed_sdhci: Introduce Capabilities Register 2 for SD slot 0 and 1
[PATCH v1 7/8] hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and AST2500 EVBs
[PATCH v1 8/8] aspeed: Support create flash devices via command line for AST1030