"Subrahmanya, Chaithrika" <[email protected]> writes:

[...]

>> > @@ -418,6 +423,11 @@ static int at24_setup(struct at24_iface *iface,
>> void *context)
>> >                printk(KERN_INFO "Read MAC addr from EEPROM: %s\n",
>> >                       print_mac(mac_str, mac_addr));
>> >
>> > +             pdev = dm644x_get_emac_pdev();
>> > +             pdata = pdev->dev.platform_data;
>> > +             pdata->phy_mask = DAVINCI_EVM_PHY_MASK;
>> > +             pdata->mdio_max_freq = EMAC_EVM_MDIO_FREQUENCY;
>> > +
>> >                davinci_init_emac(mac_addr);
>> >        }
>>
>> at24_setup() is not the right place for this.  For example, if no
>> EEPROM is detected then these values will not be setup correctly.
>> These should be set separately from the EEPROM setup.
>>
>> I think that a reasonable default should be set in the dm<soc>.c file
>> and then a way for the board code to override the defaults. I'm not
>> crazy about the dm<soc>_get_emac_pdev().
>>
>> I think I would rather see davinci_init_emac() expanded to take some
>> sort of 'emac_init' struct which contains the MAC, and these new
>> parameters.  If the values are non-zero (or not -1) then
>> davinci_init_emac() updates the pdata before the
>> platform_device_register().
>>
>
> Thank you for reviewing the patches.
> Developing on your suggestion, the following could be implemented:
> - The emac_platform_data structure for a board/EVM will be defined in
>    the board_xxx.c file.
> - The board/evm init function will call davinci_init_emac() with
>    platform data pointer being passed as a parameter.
> - The <soc>_init_emac() function in the <soc>.c file will populate
>    the necessary members of the platform data and register the
>    platform device.
> - at24_setup() will only read the MAC address and copy it to the platform
>    data structure and not call the davinci_init_emac().
> In this case, there is no need of a late_initcall for emac.
> Let me know your opinion.

I think this is a good solution.

Thanks,

Kevin

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to