| Message ID | 1316020387-6252-1-git-send-email-galak@kernel.crashing.org |
|---|---|
| State | Accepted |
| Commit | 69a852425883a4abd8dc726da34e3149a08ee95d |
| Delegated to: | Kumar Gala |
| Headers | show |
Dear Kumar Gala, In message <1316020387-6252-1-git-send-email-galak@kernel.crashing.org> you wrote: > The SoC configuration may have more ports enabled than a given board > actually can utilize. Add a routinue that allows the board code to > disable a port that it knows isn't being used. I don't get it. If you know you are not using / able to use a specific port, then why do you enable it in the first place? First creating a mess and then needing extra code to clean it up seems .... well, let's say suboptimal to me. Best regards, Wolfgang Denk
On Sep 14, 2011, at 3:13 PM, Wolfgang Denk wrote: > Dear Kumar Gala, > > In message <1316020387-6252-1-git-send-email-galak@kernel.crashing.org> you wrote: >> The SoC configuration may have more ports enabled than a given board >> actually can utilize. Add a routinue that allows the board code to >> disable a port that it knows isn't being used. > > I don't get it. If you know you are not using / able to use a > specific port, then why do you enable it in the first place? > > First creating a mess and then needing extra code to clean it up seems > .... well, let's say suboptimal to me. > > Best regards, > > Wolfgang Denk :) An example is you pick a SERDES configuration that has 8 gig-e ports. You make the choice because you might need other SERDES options like PCIe that the configuration gives you. However you only wire up 2 or 4 of those 8 gig-e ports. The SoC still thinks its configured with 8-gige ports. Now you could say that we should deal with this via #defines. Which is true, however our reference boards allow for running in hundreds if not thousands of various possible HW configurations and we've tried to avoid having different builds for such things unless completely unavoidable. So this gives the option to the board code to disable things if it knows based on some other information. In the specific case for the P3060QDS board, we have "add-on" cards that provide 4-ports of 1-gig SGMII. We can detect if one of these cards is in a slot. If the card is missing we can 'disable' the ports associated with the slot. - k
Dear Kumar Gala, In message <D144BA6D-B90C-46CD-BE7F-6197015676BB@kernel.crashing.org> you wrote: > > Now you could say that we should deal with this via #defines. Which is > true, however our reference boards allow for running in hundreds if not > thousands of various possible HW configurations and we've tried to avoid > having different builds for such things unless completely unavoidable. Understood and agreed. > So this gives the option to the board code to disable things if it knows > based on some other information. In the specific case for the P3060QDS > board, we have "add-on" cards that provide 4-ports of 1-gig SGMII. We > can detect if one of these cards is in a slot. If the card is missing > we can 'disable' the ports associated with the slot. Well, can you not turn this logic around, and come up in a configuration with all ports disabled, and then enable only those ports that are actually present? That appears to make much more sense to me. Eventually it is even more efficient in terms of things likepower cinsumption etc. ? [In the long run (maybe not so long actually) you can even do this device tree based ...] Best regards, Wolfgang Denk
On Sep 15, 2011, at 12:11 AM, Wolfgang Denk wrote: > Dear Kumar Gala, > > In message <D144BA6D-B90C-46CD-BE7F-6197015676BB@kernel.crashing.org> you wrote: >> >> Now you could say that we should deal with this via #defines. Which is >> true, however our reference boards allow for running in hundreds if not >> thousands of various possible HW configurations and we've tried to avoid >> having different builds for such things unless completely unavoidable. > > Understood and agreed. > >> So this gives the option to the board code to disable things if it knows >> based on some other information. In the specific case for the P3060QDS >> board, we have "add-on" cards that provide 4-ports of 1-gig SGMII. We >> can detect if one of these cards is in a slot. If the card is missing >> we can 'disable' the ports associated with the slot. > > Well, can you not turn this logic around, and come up in a > configuration with all ports disabled, and then enable only those > ports that are actually present? That appears to make much more sense > to me. Eventually it is even more efficient in terms of things > likepower cinsumption etc. ? Unfortuantely not. The HW makes the assumption you turn things off, not on. Once we turn something 'off' in the HW we can't turn it back 'on' as the SoC will gate the power to the block and only a hard reset will bring it back up. > [In the long run (maybe not so long actually) you can even do this > device tree based …] - k
On Sep 14, 2011, at 12:13 PM, Kumar Gala wrote: > The SoC configuration may have more ports enabled than a given board > actually can utilize. Add a routinue that allows the board code to > disable a port that it knows isn't being used. > > fm_disable_port() needs to be called before cpu_eth_init(). > > Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > --- > drivers/net/fm/fm.h | 1 + > drivers/net/fm/init.c | 8 ++++++++ > drivers/net/fm/p1023.c | 6 ++++++ > drivers/net/fm/p4080.c | 6 ++++++ > drivers/net/fm/p5020.c | 6 ++++++ > include/fm_eth.h | 1 + > 6 files changed, 28 insertions(+), 0 deletions(-) applied to 85xx - k
diff --git a/drivers/net/fm/fm.h b/drivers/net/fm/fm.h index be6714f..228df33 100644 --- a/drivers/net/fm/fm.h +++ b/drivers/net/fm/fm.h @@ -110,6 +110,7 @@ u32 fm_muram_base(int fm_idx); int fm_init_common(int index, struct ccsr_fman *reg); int fm_eth_initialize(struct ccsr_fman *reg, struct fm_eth_info *info); phy_interface_t fman_port_enet_if(enum fm_port port); +void fman_disable_port(enum fm_port port); struct fsl_enet_mac { void *base; /* MAC controller registers base address */ diff --git a/drivers/net/fm/init.c b/drivers/net/fm/init.c index 5f05ab1..512d7dd 100644 --- a/drivers/net/fm/init.c +++ b/drivers/net/fm/init.c @@ -123,6 +123,14 @@ void fman_enet_init(void) return ; } +void fm_disable_port(enum fm_port port) +{ + int i = fm_port_to_index(port); + + fm_info[i].enabled = 0; + fman_disable_port(port); +} + void fm_info_set_mdio(enum fm_port port, struct mii_dev *bus) { int i = fm_port_to_index(port); diff --git a/drivers/net/fm/p1023.c b/drivers/net/fm/p1023.c index c196e79..b17dc40 100644 --- a/drivers/net/fm/p1023.c +++ b/drivers/net/fm/p1023.c @@ -36,6 +36,12 @@ static int is_device_disabled(enum fm_port port) return port_to_devdisr[port] & devdisr; } +void fman_disable_port(enum fm_port port) +{ + ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); + setbits_be32(&gur->devdisr, port_to_devdisr[port]); +} + phy_interface_t fman_port_enet_if(enum fm_port port) { ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); diff --git a/drivers/net/fm/p4080.c b/drivers/net/fm/p4080.c index 6761a2f..791caab 100644 --- a/drivers/net/fm/p4080.c +++ b/drivers/net/fm/p4080.c @@ -44,6 +44,12 @@ static int is_device_disabled(enum fm_port port) return port_to_devdisr[port] & devdisr2; } +void fman_disable_port(enum fm_port port) +{ + ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); + setbits_be32(&gur->devdisr2, port_to_devdisr[port]); +} + phy_interface_t fman_port_enet_if(enum fm_port port) { ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); diff --git a/drivers/net/fm/p5020.c b/drivers/net/fm/p5020.c index 59638eb..69c27d2 100644 --- a/drivers/net/fm/p5020.c +++ b/drivers/net/fm/p5020.c @@ -40,6 +40,12 @@ static int is_device_disabled(enum fm_port port) return port_to_devdisr[port] & devdisr2; } +void fman_disable_port(enum fm_port port) +{ + ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); + setbits_be32(&gur->devdisr2, port_to_devdisr[port]); +} + phy_interface_t fman_port_enet_if(enum fm_port port) { ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); diff --git a/include/fm_eth.h b/include/fm_eth.h index 2ca584a..c7c6882 100644 --- a/include/fm_eth.h +++ b/include/fm_eth.h @@ -110,5 +110,6 @@ void fdt_fixup_fman_ethernet(void *fdt); phy_interface_t fm_info_get_enet_if(enum fm_port port); void fm_info_set_phy_address(enum fm_port port, int address); void fm_info_set_mdio(enum fm_port port, struct mii_dev *bus); +void fm_disable_port(enum fm_port port); #endif
The SoC configuration may have more ports enabled than a given board actually can utilize. Add a routinue that allows the board code to disable a port that it knows isn't being used. fm_disable_port() needs to be called before cpu_eth_init(). Signed-off-by: Kumar Gala <galak@kernel.crashing.org> --- drivers/net/fm/fm.h | 1 + drivers/net/fm/init.c | 8 ++++++++ drivers/net/fm/p1023.c | 6 ++++++ drivers/net/fm/p4080.c | 6 ++++++ drivers/net/fm/p5020.c | 6 ++++++ include/fm_eth.h | 1 + 6 files changed, 28 insertions(+), 0 deletions(-)