| Message ID | 1386350983-13281-3-git-send-email-wens@csie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
From: Chen-Yu Tsai <wens@csie.org> Date: Sat, 7 Dec 2013 01:29:35 +0800 > @@ -47,6 +47,7 @@ static int stmmac_probe_config_dt(struct platform_device *pdev, > plat->bus_id = 0; > > of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr); > + plat->force_sf_dma_mode = of_property_read_bool(np, "snps,force_sf_dma_mode"); Will this do the right thing for when the property is not present? Right now the force_sf_dma_mode value is always false. In fact won't it override the explicit settings done elsewhere in the driver?
Resending reply due to incorrect MIME type. On Sat, Dec 7, 2013 at 5:26 AM, David Miller <davem@davemloft.net> wrote: > > From: Chen-Yu Tsai <wens@csie.org> > Date: Sat, 7 Dec 2013 01:29:35 +0800 > > > @@ -47,6 +47,7 @@ static int stmmac_probe_config_dt(struct platform_device *pdev, > > plat->bus_id = 0; > > > > of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr); > > + plat->force_sf_dma_mode = of_property_read_bool(np, "snps,force_sf_dma_mode"); > > Will this do the right thing for when the property is not present? > Right now the force_sf_dma_mode value is always false. of_property_read_bool will return false when the property is not present. > In fact won't it override the explicit settings done elsewhere in the > driver? Point taken. The current implementation will override settings passed from platform data. ORing the two would be better. Thanks
On Sat, Dec 07, 2013 at 09:23:27AM +0800, Chen-Yu Tsai wrote: > Point taken. The current implementation will override settings passed from > platform data. ORing the two would be better. Platform_data and DT-based configuration are pretty unlikely to be used together, so ORing it doesn't have much sense. Maxime
On Saturday 07 of December 2013 11:07:37 maxime.ripard wrote: > On Sat, Dec 07, 2013 at 09:23:27AM +0800, Chen-Yu Tsai wrote: > > Point taken. The current implementation will override settings passed from > > platform data. ORing the two would be better. > > Platform_data and DT-based configuration are pretty unlikely to be > used together, so ORing it doesn't have much sense. In fact, the recommended way is to always use platform data alone if it is present or try to parse DT otherwise, so no mixing of data from these two sources should be done. Best regards, Tomasz
On Sat, Dec 7, 2013 at 7:06 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On Saturday 07 of December 2013 11:07:37 maxime.ripard wrote: >> On Sat, Dec 07, 2013 at 09:23:27AM +0800, Chen-Yu Tsai wrote: >> > Point taken. The current implementation will override settings passed from >> > platform data. ORing the two would be better. >> >> Platform_data and DT-based configuration are pretty unlikely to be >> used together, so ORing it doesn't have much sense. > > In fact, the recommended way is to always use platform data alone if it is > present or try to parse DT otherwise, so no mixing of data from these two > sources should be done. Would binding platform data with compatibles, as I did so in this patch series, be a bad idea then?
On Mon, Dec 09, 2013 at 10:59:38AM +0800, Chen-Yu Tsai wrote: > On Sat, Dec 7, 2013 at 7:06 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > > On Saturday 07 of December 2013 11:07:37 maxime.ripard wrote: > >> On Sat, Dec 07, 2013 at 09:23:27AM +0800, Chen-Yu Tsai wrote: > >> > Point taken. The current implementation will override settings passed from > >> > platform data. ORing the two would be better. > >> > >> Platform_data and DT-based configuration are pretty unlikely to be > >> used together, so ORing it doesn't have much sense. > > > > In fact, the recommended way is to always use platform data alone if it is > > present or try to parse DT otherwise, so no mixing of data from these two > > sources should be done. > > Would binding platform data with compatibles, as I did so in this patch > series, be a bad idea then? What I meant was that you'll either be probed will pdev->dev.of_node or pdev->dev.platform_data filled, but not both at the same time, so ORing it isn't really sensible. But I don't see why you couldn't reuse the stmmac_platform_data structure in your patch. Maxime
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index 51c9069..74c7aef 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -47,6 +47,7 @@ static int stmmac_probe_config_dt(struct platform_device *pdev, plat->bus_id = 0; of_property_read_u32(np, "snps,phy-addr", &plat->phy_addr); + plat->force_sf_dma_mode = of_property_read_bool(np, "snps,force_sf_dma_mode"); plat->mdio_bus_data = devm_kzalloc(&pdev->dev, sizeof(struct stmmac_mdio_bus_data),
"snps,force_sf_dma_mode" is documented in stmmac device tree bindings, but is never handled by the driver. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 + 1 file changed, 1 insertion(+)