On Thu, Feb 24, 2011 at 4:29 PM, Cousson, Benoit <b-cousson@xxxxxx> wrote:
> On 2/23/2011 6:47 PM, Kadiyala, Kishore wrote:
>>
>> Add a device attribute to hwmod data of omap2430, omap3, omap4.
>> Currently the device attribute holds information regarding dual volt MMC
>> card
>> support by the controller which will be later passed to the host driver
>> via
>> platform data.
>>
>> Signed-off-by: Kevin Hilman<khilman@xxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Kishore Kadiyala<kishore.kadiyala@xxxxxx>
>> Cc: Benoit Cousson<b-cousson@xxxxxx>
>> Cc: Paul Walmsley<paul@xxxxxxxxx>
>
> There are few minor comments to fix hence the:
> Almost-Acked-by: Benoit Cousson<b-cousson@xxxxxx>
>
>
>> ---
>> arch/arm/mach-omap2/omap_hwmod_2430_data.c | 9 +++++++++
>> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 12 ++++++++++++
>> arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 16 ++++++++++++++++
>> arch/arm/plat-omap/include/plat/mmc.h | 10 ++++++++++
>> drivers/mmc/host/omap_hsmmc.c | 4 ++--
>> 5 files changed, 49 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> index 4d45b7d..e050355 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
>> @@ -19,6 +19,7 @@
>> #include<plat/i2c.h>
>> #include<plat/gpio.h>
>> #include<plat/mcspi.h>
>> +#include<plat/mmc.h>
>>
>> #include "omap_hwmod_common_data.h"
>>
>> @@ -1290,6 +1291,10 @@ static struct omap_hwmod_class mmc_class = {
>>
>> /* MMC/SD/SDIO1 */
>>
>> +static struct mmc_dev_attr mmc1_dev_attr = {
>
> Could you rename the struct omap_mmc_dev_attr to stick to the convention?
Ok
>
> The dev_attr should be just above "static struct omap_hwmod". See the OMAP4
> example below.
>
>> + .flags = OMAP_HSMMC_SUPPORTS_DUAL_VOLT,
>> +};
>> +
>> static struct omap_hwmod_irq_info mmc1_mpu_irqs[] = {
>> { .irq = 83 },
>> };
>> @@ -1328,11 +1333,14 @@ static struct omap_hwmod omap2430_mmc1_hwmod = {
>> .slaves = omap2430_mmc1_slaves,
>> .slaves_cnt = ARRAY_SIZE(omap2430_mmc1_slaves),
>> .class =&mmc_class,
>> + .dev_attr =&mmc1_dev_attr,
>
> dev_attr should be above .slaves.
>
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP2430),
>> };
>>
>> /* MMC/SD/SDIO2 */
>>
>> +static struct mmc_dev_attr mmc2_dev_attr;
>
> If you do not have any dev_attr for that instance, do not add it here and
> test for null pointer in your driver.
> This comment applies for all instances in all OMAPs.
ok
>
>> +
>> static struct omap_hwmod_irq_info mmc2_mpu_irqs[] = {
>> { .irq = 86 },
>> };
>> @@ -1371,6 +1379,7 @@ static struct omap_hwmod omap2430_mmc2_hwmod = {
>> .slaves = omap2430_mmc2_slaves,
>> .slaves_cnt = ARRAY_SIZE(omap2430_mmc2_slaves),
>> .class =&mmc_class,
>> + .dev_attr =&mmc2_dev_attr,
>
> Not needed if there is not data in the structure.
ok
>
> [...]
>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> index dd39e75..6c4ccfd 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> @@ -25,6 +25,7 @@
>> #include<plat/gpio.h>
>> #include<plat/dma.h>
>> #include<plat/mcspi.h>
>> +#include<plat/mmc.h>
>>
>> #include "omap_hwmod_common_data.h"
>>
>> @@ -3383,6 +3384,10 @@ static struct omap_hwmod_class
>> omap44xx_mmc_hwmod_class = {
>> };
>>
>> /* mmc1 */
>> +static struct mmc_dev_attr omap_mmc1_dev_attr = {
>> + .flags = OMAP_HSMMC_SUPPORTS_DUAL_VOLT,
>> +};
>> +
>> static struct omap_hwmod_irq_info omap44xx_mmc1_irqs[] = {
>> { .irq = 83 + OMAP44XX_IRQ_GIC_START },
>> };
>> @@ -3437,10 +3442,14 @@ static struct omap_hwmod omap44xx_mmc1_hwmod = {
>> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc1_slaves),
>> .masters = omap44xx_mmc1_masters,
>> .masters_cnt = ARRAY_SIZE(omap44xx_mmc1_masters),
>> + .dev_attr =&omap_mmc1_dev_attr,
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>> };
>>
>> /* mmc2 */
>> +static struct omap_hwmod omap44xx_mmc2_hwmod;
>> +static struct mmc_dev_attr omap_mmc2_dev_attr;
>> +
>> static struct omap_hwmod_irq_info omap44xx_mmc2_irqs[] = {
>> { .irq = 86 + OMAP44XX_IRQ_GIC_START },
>> };
>> @@ -3495,11 +3504,13 @@ static struct omap_hwmod omap44xx_mmc2_hwmod = {
>> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc2_slaves),
>> .masters = omap44xx_mmc2_masters,
>> .masters_cnt = ARRAY_SIZE(omap44xx_mmc2_masters),
>> + .dev_attr =&omap_mmc2_dev_attr,
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>> };
>>
>> /* mmc3 */
>> static struct omap_hwmod omap44xx_mmc3_hwmod;
>> +static struct mmc_dev_attr omap_mmc3_dev_attr;
>> static struct omap_hwmod_irq_info omap44xx_mmc3_irqs[] = {
>> { .irq = 94 + OMAP44XX_IRQ_GIC_START },
>> };
>> @@ -3547,11 +3558,13 @@ static struct omap_hwmod omap44xx_mmc3_hwmod = {
>> },
>> .slaves = omap44xx_mmc3_slaves,
>> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc3_slaves),
>> + .dev_attr =&omap_mmc3_dev_attr,
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>> };
>>
>> /* mmc4 */
>> static struct omap_hwmod omap44xx_mmc4_hwmod;
>> +static struct mmc_dev_attr omap_mmc4_dev_attr;
>> static struct omap_hwmod_irq_info omap44xx_mmc4_irqs[] = {
>> { .irq = 96 + OMAP44XX_IRQ_GIC_START },
>> };
>> @@ -3599,11 +3612,13 @@ static struct omap_hwmod omap44xx_mmc4_hwmod = {
>> },
>> .slaves = omap44xx_mmc4_slaves,
>> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc4_slaves),
>> + .dev_attr =&omap_mmc4_dev_attr,
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>> };
>>
>> /* mmc5 */
>> static struct omap_hwmod omap44xx_mmc5_hwmod;
>> +static struct mmc_dev_attr omap_mmc5_dev_attr;
>> static struct omap_hwmod_irq_info omap44xx_mmc5_irqs[] = {
>> { .irq = 59 + OMAP44XX_IRQ_GIC_START },
>> };
>> @@ -3651,6 +3666,7 @@ static struct omap_hwmod omap44xx_mmc5_hwmod = {
>> },
>> .slaves = omap44xx_mmc5_slaves,
>> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc5_slaves),
>> + .dev_attr =&omap_mmc5_dev_attr,
>> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
>> };
>>
>
> In order to be aligned with the generator, and assuming that only the mm1
> needs dev_attr, the OMAP4 diff should be:
ok, will have changes as below.
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 79a8601..958651c 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -3420,6 +3420,11 @@ static struct omap_hwmod_ocp_if
> *omap44xx_mmc1_slaves[] = {
> &omap44xx_l4_per__mmc1,
> };
>
> +/* mmc1 dev_attr */
> +static struct omap_mmc_dev_attr mmc1_dev_attr = {
> + .flags = OMAP_HSMMC_SUPPORTS_DUAL_VOLT,
> +};
> +
> static struct omap_hwmod omap44xx_mmc1_hwmod = {
> .name = "mmc1",
> .class = &omap44xx_mmc_hwmod_class,
> @@ -3433,6 +3438,7 @@ static struct omap_hwmod omap44xx_mmc1_hwmod = {
> .clkctrl_reg = OMAP4430_CM_L3INIT_MMC1_CLKCTRL,
> },
> },
> + .dev_attr = &mmc1_dev_attr,
> .slaves = omap44xx_mmc1_slaves,
> .slaves_cnt = ARRAY_SIZE(omap44xx_mmc1_slaves),
> .masters = omap44xx_mmc1_masters,
> ---
>
> Regards,
> Benoit
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
