Hi, On Fri, Dec 13, 2013 at 6:38 PM, Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: > On Thu, Dec 12, 2013 at 06:31:43PM +0800, Chen-Yu Tsai wrote: >> Hi, >> >> On Thu, Dec 12, 2013 at 5:04 PM, Maxime Ripard >> <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: >> > Hi, >> > >> > On Wed, Dec 11, 2013 at 02:45:08PM +0000, srinivas kandagatla wrote: >> >> >>> 1. .tx_coe >> >> >>> This is not exported in the DT bindings. >> >> >>> Looking at stmmac code, not setting this seems to disable all >> >> >>> checksum offloading. >> >> >> >> >> >> Why cant this go via DT as well? >> >> > >> >> > If you and Giuseppe are OK with this, why not? >> >> Am Ok with it. >> > >> > Please note that I'm opposed to this until someone explain why putting >> > it in the DT is relevant (and not just convenient). >> >> Checksum offloading is an optional feature[1], implemented starting >> from version 3.20a. It is not tied to a specific IP version. As such, >> using a "snps,dwmac-<version>" compatible isn't a good fit here. > > No, but we're not in such case. Since we have a compatible of our own, > we can derive it from that. Putting a property in the DT would only be > redundant. > >> stmmac does auto-detection for optional features on MAC version > 3.50a. >> This is what Srinivas was referring to. >> >> Unfortunately, our MAC is < 3.50a. No auto-detection. We could add a >> "snps,dwmac-tx-coe" compatible for this, or the seperate DT property. >> >> The other way would be to pass the flags in the initial .data with the >> SoC specific compatible. Other SoCs with the same feature won't be >> able to reuse the same compatible though. > > Which is already pretty much the case, since we have to deal with > Allwinner specific code and features. > > A new compatible is cheap to maintain, a new property is not. 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. 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); 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); Cheers, ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html