Hi Chen,
On 24/12/13 03:27, Chen-Yu Tsai wrote:
> Srinivas,
>
> Let's keep platform data as of_data, so SoC compatibles can pass
> hardware feature flags for cores that don't support auto-detection.
I understand your concern here, But making platform_data as of_data
would just open a wide door for other hacky stuff too. So other glue
drivers would just use this interface instead, ignoring standard
interfaces. Also there would be issues on who takes the priority if the
parameters are specified in multiple places.
Can't this field/s be one of the variable in the struct stmmac_of_data
rather than reusing platform data?
>
> We can adjust the callbacks as you suggested, and I added a .free
> to complement .setup. Driver private data and platform data are
> off limits to the callbacks. My version of the callbacks:
>
> void (*fix_mac_speed)(void *priv, unsigned int speed);
> void (*bus_setup)(void __iomem *ioaddr);
In reply to your question in last email:
bus_setup this is something very specific to ST which configures the bus
parameters of AMBA-to-STBUS converter. This register falls inside the
memory map of stmmac.
> void *(*setup)(struct platform_device *pdev);
> void (*free)(struct platform_device *pdev, void *priv);
> int (*init)(struct platform_device *pdev, void *priv);
> void (*exit)(struct platform_device *pdev, void *priv);
>
The callbacks to Ok to me, unless Peppe has more comments on this.
Thanks,
srini
>
> Cheers,
>
> ChenYu
Hi,
On Thu, Jan 2, 2014 at 9:11 PM, srinivas kandagatla
<[email protected]> wrote:
> Hi Chen,
>
> On 24/12/13 03:27, Chen-Yu Tsai wrote:
>> Srinivas,
>>
>> Let's keep platform data as of_data, so SoC compatibles can pass
>> hardware feature flags for cores that don't support auto-detection.
>
> I understand your concern here, But making platform_data as of_data
> would just open a wide door for other hacky stuff too. So other glue
> drivers would just use this interface instead, ignoring standard
> interfaces. Also there would be issues on who takes the priority if the
> parameters are specified in multiple places.
>
>
> Can't this field/s be one of the variable in the struct stmmac_of_data
> rather than reusing platform data?
We (sunxi) need .has_gmac and .tx_coe.
To be generic and usable to other SoCs, it seems like I would be
adding most of the fields from platform data. But maybe future
glue layers will only need the callbacks. I can not be sure.
Also since snps,phy-addr should be deprecated, I am thinking of
changing the default phy address to auto-detect, until
Giuseppe merges his phy node support work.
I will post a v2 series this week.
>>
>> We can adjust the callbacks as you suggested, and I added a .free
>> to complement .setup. Driver private data and platform data are
>> off limits to the callbacks. My version of the callbacks:
>>
>> void (*fix_mac_speed)(void *priv, unsigned int speed);
>> void (*bus_setup)(void __iomem *ioaddr);
>
> In reply to your question in last email:
> bus_setup this is something very specific to ST which configures the bus
> parameters of AMBA-to-STBUS converter. This register falls inside the
> memory map of stmmac.
I see. IMHO it could be merged into .init?
>
>> void *(*setup)(struct platform_device *pdev);
>> void (*free)(struct platform_device *pdev, void *priv);
>> int (*init)(struct platform_device *pdev, void *priv);
>> void (*exit)(struct platform_device *pdev, void *priv);
>>
>
> The callbacks to Ok to me, unless Peppe has more comments on this.
Ok.
Cheers
ChenYu