| Message ID | 20200603145258.195920-3-linux@roeck-us.net |
|---|---|
| State | New |
| Headers | show |
| Series | sd: sdhci: Implement basic vendor specific register support | expand |
On 6/3/20 4:52 PM, Guenter Roeck wrote: > Set vendor property to IMX to enable IMX specific functionality > in sdhci code. > > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Added missing error checks > Added Philippe's Tested-by: tag > > hw/arm/fsl-imx25.c | 6 ++++++ > hw/arm/fsl-imx6.c | 6 ++++++ > hw/arm/fsl-imx6ul.c | 2 ++ > hw/arm/fsl-imx7.c | 2 ++ > 4 files changed, 16 insertions(+) > > diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c > index cdaa79c26b..a853ffcc00 100644 > --- a/hw/arm/fsl-imx25.c > +++ b/hw/arm/fsl-imx25.c > @@ -274,6 +274,12 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) > &err); > object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES, > "capareg", &err); > + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, > + "vendor", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err); > if (err) { > error_propagate(errp, err); > diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c > index f58c85aa8c..29677cfd59 100644 > --- a/hw/arm/fsl-imx6.c > +++ b/hw/arm/fsl-imx6.c > @@ -350,6 +350,12 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp) > &err); > object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES, > "capareg", &err); > + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, > + "vendor", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err); > if (err) { > error_propagate(errp, err); > diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c > index 3ecb212da6..ce1462927c 100644 > --- a/hw/arm/fsl-imx6ul.c > +++ b/hw/arm/fsl-imx6ul.c > @@ -505,6 +505,8 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp) > FSL_IMX6UL_USDHC2_IRQ, > }; > > + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX, > + "vendor", &error_abort); > object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized", > &error_abort); > > diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c > index 89c3b64c06..dbf16b2814 100644 > --- a/hw/arm/fsl-imx7.c > +++ b/hw/arm/fsl-imx7.c > @@ -416,6 +416,8 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp) > FSL_IMX7_USDHC3_IRQ, > }; > > + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX, > + "vendor", &error_abort); > object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized", > &error_abort); > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On Wed, 3 Jun 2020 at 15:53, Guenter Roeck <linux@roeck-us.net> wrote: > > Set vendor property to IMX to enable IMX specific functionality > in sdhci code. > > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v2: Added missing error checks > Added Philippe's Tested-by: tag > > hw/arm/fsl-imx25.c | 6 ++++++ > hw/arm/fsl-imx6.c | 6 ++++++ > hw/arm/fsl-imx6ul.c | 2 ++ > hw/arm/fsl-imx7.c | 2 ++ > 4 files changed, 16 insertions(+) > > diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c > index cdaa79c26b..a853ffcc00 100644 > --- a/hw/arm/fsl-imx25.c > +++ b/hw/arm/fsl-imx25.c > @@ -274,6 +274,12 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) > &err); > object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES, > "capareg", &err); > + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, > + "vendor", &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } The existing error handling in this function is wrong -- for multiple calls that take a pointer to a local Error*, the check-and-error_propagate() has to be done after each call, it can't be rolled up into a single check after all the calls. (see include/qapi/error.h for the patterns that are valid). On the other hand this change is correct-in-itself so I guess it isn't making the problem worse... thanks -- PMM
On 6/15/20 6:18 AM, Peter Maydell wrote: > On Wed, 3 Jun 2020 at 15:53, Guenter Roeck <linux@roeck-us.net> wrote: >> >> Set vendor property to IMX to enable IMX specific functionality >> in sdhci code. >> >> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> v2: Added missing error checks >> Added Philippe's Tested-by: tag >> >> hw/arm/fsl-imx25.c | 6 ++++++ >> hw/arm/fsl-imx6.c | 6 ++++++ >> hw/arm/fsl-imx6ul.c | 2 ++ >> hw/arm/fsl-imx7.c | 2 ++ >> 4 files changed, 16 insertions(+) >> >> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c >> index cdaa79c26b..a853ffcc00 100644 >> --- a/hw/arm/fsl-imx25.c >> +++ b/hw/arm/fsl-imx25.c >> @@ -274,6 +274,12 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) >> &err); >> object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES, >> "capareg", &err); >> + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, >> + "vendor", &err); >> + if (err) { >> + error_propagate(errp, err); >> + return; >> + } > > The existing error handling in this function is wrong -- for multiple > calls that take a pointer to a local Error*, the > check-and-error_propagate() has to be done after each call, it can't > be rolled up into a single check after all the calls. (see > include/qapi/error.h for the patterns that are valid). > A fix for that problem was submitted by Philippe: https://www.mail-archive.com/qemu-devel@nongnu.org/msg695544.html Thanks, Guenter
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c index cdaa79c26b..a853ffcc00 100644 --- a/hw/arm/fsl-imx25.c +++ b/hw/arm/fsl-imx25.c @@ -274,6 +274,12 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) &err); object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES, "capareg", &err); + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, + "vendor", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c index f58c85aa8c..29677cfd59 100644 --- a/hw/arm/fsl-imx6.c +++ b/hw/arm/fsl-imx6.c @@ -350,6 +350,12 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp) &err); object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES, "capareg", &err); + object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX, + "vendor", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c index 3ecb212da6..ce1462927c 100644 --- a/hw/arm/fsl-imx6ul.c +++ b/hw/arm/fsl-imx6ul.c @@ -505,6 +505,8 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp) FSL_IMX6UL_USDHC2_IRQ, }; + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX, + "vendor", &error_abort); object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized", &error_abort); diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c index 89c3b64c06..dbf16b2814 100644 --- a/hw/arm/fsl-imx7.c +++ b/hw/arm/fsl-imx7.c @@ -416,6 +416,8 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp) FSL_IMX7_USDHC3_IRQ, }; + object_property_set_uint(OBJECT(&s->usdhc[i]), SDHCI_VENDOR_IMX, + "vendor", &error_abort); object_property_set_bool(OBJECT(&s->usdhc[i]), true, "realized", &error_abort);