From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755913AbcBVVwV (ORCPT ); Mon, 22 Feb 2016 16:52:21 -0500 Received: from mail-io0-f170.google.com ([209.85.223.170]:35089 "EHLO mail-io0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755859AbcBVVwS (ORCPT ); Mon, 22 Feb 2016 16:52:18 -0500 MIME-Version: 1.0 In-Reply-To: References: <1454511275-9791-1-git-send-email-alexandre.torgue@gmail.com> <1454511275-9791-2-git-send-email-alexandre.torgue@gmail.com> Date: Mon, 22 Feb 2016 22:52:17 +0100 Message-ID: Subject: Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip From: Joachim Eastwood To: Alexandre Torgue Cc: Maxime Coquelin , Giuseppe Cavallaro , netdev , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22 February 2016 at 15:50, Alexandre Torgue wrote: > 2016-02-13 14:48 GMT+01:00 Joachim Eastwood : >> On 3 February 2016 at 15:54, Alexandre TORGUE >> wrote: >>> + plat_dat->bsp_priv = dwmac; >>> + plat_dat->init = stm32_dwmac_init; >>> + plat_dat->exit = stm32_dwmac_exit; >> >> Instead of using these callbacks could you rather implement the PM >> callbacks directly in this driver? >> I don't think it should add much code and it will make it look more >> like standard driver. This will also give you some more control and >> flexibility in your code. > > I prefer to keep the code as it is. Glue layer is directly linked to > stmmac driver and I don't want to brake the link between the glue and > the stmmac driver. What do you mean by break the link? There has been numerous of patch sets to make the stmmac "glue" drivers into more standard platform drivers. http://marc.info/?l=linux-netdev&m=143159850631093&w=2 http://marc.info/?l=linux-netdev&m=143708560009851&w=2 http://marc.info/?l=linux-netdev&m=143812136600541&w=2 Do you see any advantage by using the init and exit hooks in your driver instead of using the standard driver PM callbacks and remove function? The only "cost" I see is slightly more boilerplate code. But since you already have init/exit functions you could easily make them into PM resume/suspend so I doubt there would be much increase in code size. One other thing; Do you need to have the PHY mode setup code in the init function which is called each time on resume? If you could move it to probe you could drop the interface priv data member and use plat_dat->interface as stmmac_probe_config_dt() has already done of_get_phy_mode(). regards, Joachim Eastwood From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joachim Eastwood Subject: Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip Date: Mon, 22 Feb 2016 22:52:17 +0100 Message-ID: References: <1454511275-9791-1-git-send-email-alexandre.torgue@gmail.com> <1454511275-9791-2-git-send-email-alexandre.torgue@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Maxime Coquelin , Giuseppe Cavallaro , netdev , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" To: Alexandre Torgue Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 22 February 2016 at 15:50, Alexandre Torgue wrote: > 2016-02-13 14:48 GMT+01:00 Joachim Eastwood : >> On 3 February 2016 at 15:54, Alexandre TORGUE >> wrote: >>> + plat_dat->bsp_priv = dwmac; >>> + plat_dat->init = stm32_dwmac_init; >>> + plat_dat->exit = stm32_dwmac_exit; >> >> Instead of using these callbacks could you rather implement the PM >> callbacks directly in this driver? >> I don't think it should add much code and it will make it look more >> like standard driver. This will also give you some more control and >> flexibility in your code. > > I prefer to keep the code as it is. Glue layer is directly linked to > stmmac driver and I don't want to brake the link between the glue and > the stmmac driver. What do you mean by break the link? There has been numerous of patch sets to make the stmmac "glue" drivers into more standard platform drivers. http://marc.info/?l=linux-netdev&m=143159850631093&w=2 http://marc.info/?l=linux-netdev&m=143708560009851&w=2 http://marc.info/?l=linux-netdev&m=143812136600541&w=2 Do you see any advantage by using the init and exit hooks in your driver instead of using the standard driver PM callbacks and remove function? The only "cost" I see is slightly more boilerplate code. But since you already have init/exit functions you could easily make them into PM resume/suspend so I doubt there would be much increase in code size. One other thing; Do you need to have the PHY mode setup code in the init function which is called each time on resume? If you could move it to probe you could drop the interface priv data member and use plat_dat->interface as stmmac_probe_config_dt() has already done of_get_phy_mode(). regards, Joachim Eastwood From mboxrd@z Thu Jan 1 00:00:00 1970 From: manabian@gmail.com (Joachim Eastwood) Date: Mon, 22 Feb 2016 22:52:17 +0100 Subject: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip In-Reply-To: References: <1454511275-9791-1-git-send-email-alexandre.torgue@gmail.com> <1454511275-9791-2-git-send-email-alexandre.torgue@gmail.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 22 February 2016 at 15:50, Alexandre Torgue wrote: > 2016-02-13 14:48 GMT+01:00 Joachim Eastwood : >> On 3 February 2016 at 15:54, Alexandre TORGUE >> wrote: >>> + plat_dat->bsp_priv = dwmac; >>> + plat_dat->init = stm32_dwmac_init; >>> + plat_dat->exit = stm32_dwmac_exit; >> >> Instead of using these callbacks could you rather implement the PM >> callbacks directly in this driver? >> I don't think it should add much code and it will make it look more >> like standard driver. This will also give you some more control and >> flexibility in your code. > > I prefer to keep the code as it is. Glue layer is directly linked to > stmmac driver and I don't want to brake the link between the glue and > the stmmac driver. What do you mean by break the link? There has been numerous of patch sets to make the stmmac "glue" drivers into more standard platform drivers. http://marc.info/?l=linux-netdev&m=143159850631093&w=2 http://marc.info/?l=linux-netdev&m=143708560009851&w=2 http://marc.info/?l=linux-netdev&m=143812136600541&w=2 Do you see any advantage by using the init and exit hooks in your driver instead of using the standard driver PM callbacks and remove function? The only "cost" I see is slightly more boilerplate code. But since you already have init/exit functions you could easily make them into PM resume/suspend so I doubt there would be much increase in code size. One other thing; Do you need to have the PHY mode setup code in the init function which is called each time on resume? If you could move it to probe you could drop the interface priv data member and use plat_dat->interface as stmmac_probe_config_dt() has already done of_get_phy_mode(). regards, Joachim Eastwood