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_...@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 <and...@codeconstruct.com.au>
