* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used @ 2020-02-07 11:31 Michal Simek 2020-02-10 12:07 ` Grygorii Strashko 0 siblings, 1 reply; 9+ messages in thread From: Michal Simek @ 2020-02-07 11:31 UTC (permalink / raw) To: u-boot There is no reason to check sgmii branch again when it is clear that phy interface is rgmii. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- drivers/net/phy/dp83867.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c index 4d796e289c45..3178787ff1c7 100644 --- a/drivers/net/phy/dp83867.c +++ b/drivers/net/phy/dp83867.c @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device *phydev) phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIIDCTL, delay); - } - - if (phy_interface_is_sgmii(phydev)) { + } else if (phy_interface_is_sgmii(phydev)) { phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); -- 2.25.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-07 11:31 [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used Michal Simek @ 2020-02-10 12:07 ` Grygorii Strashko 2020-02-11 8:11 ` Michal Simek 0 siblings, 1 reply; 9+ messages in thread From: Grygorii Strashko @ 2020-02-10 12:07 UTC (permalink / raw) To: u-boot On 07/02/2020 13:31, Michal Simek wrote: > There is no reason to check sgmii branch again when it is clear that phy > interface is rgmii. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > drivers/net/phy/dp83867.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c > index 4d796e289c45..3178787ff1c7 100644 > --- a/drivers/net/phy/dp83867.c > +++ b/drivers/net/phy/dp83867.c > @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device *phydev) > > phy_write_mmd(phydev, DP83867_DEVADDR, > DP83867_RGMIIDCTL, delay); > - } > - > - if (phy_interface_is_sgmii(phydev)) { > + } else if (phy_interface_is_sgmii(phydev)) { > phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, > (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); > > From one side I have no objections, but from another - I'd prefer to keep as is. -- Best regards, grygorii ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-10 12:07 ` Grygorii Strashko @ 2020-02-11 8:11 ` Michal Simek 2020-02-12 20:24 ` Grygorii Strashko 0 siblings, 1 reply; 9+ messages in thread From: Michal Simek @ 2020-02-11 8:11 UTC (permalink / raw) To: u-boot On 10. 02. 20 13:07, Grygorii Strashko wrote: > > > On 07/02/2020 13:31, Michal Simek wrote: >> There is no reason to check sgmii branch again when it is clear that phy >> interface is rgmii. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> drivers/net/phy/dp83867.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >> index 4d796e289c45..3178787ff1c7 100644 >> --- a/drivers/net/phy/dp83867.c >> +++ b/drivers/net/phy/dp83867.c >> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device *phydev) >> phy_write_mmd(phydev, DP83867_DEVADDR, >> DP83867_RGMIIDCTL, delay); >> - } >> - >> - if (phy_interface_is_sgmii(phydev)) { >> + } else if (phy_interface_is_sgmii(phydev)) { >> phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >> (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >> > > From one side I have no objections, but from another - I'd prefer to > keep as is. Can you please be elaborate on this one more? Thanks, Michal ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-11 8:11 ` Michal Simek @ 2020-02-12 20:24 ` Grygorii Strashko 2020-02-13 6:23 ` Michal Simek 0 siblings, 1 reply; 9+ messages in thread From: Grygorii Strashko @ 2020-02-12 20:24 UTC (permalink / raw) To: u-boot On 11/02/2020 10:11, Michal Simek wrote: > On 10. 02. 20 13:07, Grygorii Strashko wrote: >> >> >> On 07/02/2020 13:31, Michal Simek wrote: >>> There is no reason to check sgmii branch again when it is clear that phy >>> interface is rgmii. >>> >>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>> --- >>> >>> ? drivers/net/phy/dp83867.c | 4 +--- >>> ? 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>> index 4d796e289c45..3178787ff1c7 100644 >>> --- a/drivers/net/phy/dp83867.c >>> +++ b/drivers/net/phy/dp83867.c >>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device *phydev) >>> ? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>> ??????????????????? DP83867_RGMIIDCTL, delay); >>> -??? } >>> - >>> -??? if (phy_interface_is_sgmii(phydev)) { >>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>> ????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>> ??????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >>> >> >> From one side I have no objections, but from another - I'd prefer to >> keep as is. > > Can you please be elaborate on this one more? - keep the same way as in the Kernel - code readability -- Best regards, grygorii ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-12 20:24 ` Grygorii Strashko @ 2020-02-13 6:23 ` Michal Simek 2020-02-13 15:49 ` Grygorii Strashko 0 siblings, 1 reply; 9+ messages in thread From: Michal Simek @ 2020-02-13 6:23 UTC (permalink / raw) To: u-boot On 12. 02. 20 21:24, Grygorii Strashko wrote: > > > On 11/02/2020 10:11, Michal Simek wrote: >> On 10. 02. 20 13:07, Grygorii Strashko wrote: >>> >>> >>> On 07/02/2020 13:31, Michal Simek wrote: >>>> There is no reason to check sgmii branch again when it is clear that >>>> phy >>>> interface is rgmii. >>>> >>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>> --- >>>> >>>> ?? drivers/net/phy/dp83867.c | 4 +--- >>>> ?? 1 file changed, 1 insertion(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>>> index 4d796e289c45..3178787ff1c7 100644 >>>> --- a/drivers/net/phy/dp83867.c >>>> +++ b/drivers/net/phy/dp83867.c >>>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device >>>> *phydev) >>>> ?? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>>> ???????????????????? DP83867_RGMIIDCTL, delay); >>>> -??? } >>>> - >>>> -??? if (phy_interface_is_sgmii(phydev)) { >>>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>>> ?????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>>> ???????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >>>> ? >>> >>> ?From one side I have no objections, but from another - I'd prefer to >>> keep as is. >> >> Can you please be elaborate on this one more? > > - keep the same way as in the Kernel If kernel does it in the same way it should be also fixed. I have been checking yesterday dt binding docs in u-boot and in Linux and surprisingly they are different. ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described ti,clk-output-sel is supported but even in code is said that it is optional property. ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth are not documented in dt binding doc ti,sgmii-ref-clock-output-enable is not supported in u-boot but it is in Linux and we are using this feature. Can you please sync it if you want to keep it in the same was as is done in Linux? > - code readability I don't think this is really changing code readability. For improving readability would be the best to move bodies of these ifs to separate functions and not have dp83867_config() ~140 lines long. Thanks, Michal ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-13 6:23 ` Michal Simek @ 2020-02-13 15:49 ` Grygorii Strashko 2020-02-13 16:05 ` Michal Simek 0 siblings, 1 reply; 9+ messages in thread From: Grygorii Strashko @ 2020-02-13 15:49 UTC (permalink / raw) To: u-boot On 13/02/2020 08:23, Michal Simek wrote: > On 12. 02. 20 21:24, Grygorii Strashko wrote: >> >> >> On 11/02/2020 10:11, Michal Simek wrote: >>> On 10. 02. 20 13:07, Grygorii Strashko wrote: >>>> >>>> >>>> On 07/02/2020 13:31, Michal Simek wrote: >>>>> There is no reason to check sgmii branch again when it is clear that >>>>> phy >>>>> interface is rgmii. >>>>> >>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>> --- >>>>> >>>>> ?? drivers/net/phy/dp83867.c | 4 +--- >>>>> ?? 1 file changed, 1 insertion(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>>>> index 4d796e289c45..3178787ff1c7 100644 >>>>> --- a/drivers/net/phy/dp83867.c >>>>> +++ b/drivers/net/phy/dp83867.c >>>>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device >>>>> *phydev) >>>>> ?? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>>>> ???????????????????? DP83867_RGMIIDCTL, delay); >>>>> -??? } >>>>> - >>>>> -??? if (phy_interface_is_sgmii(phydev)) { >>>>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>>>> ?????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>>>> ???????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >>>>> >>>> >>>> ?From one side I have no objections, but from another - I'd prefer to >>>> keep as is. >>> >>> Can you please be elaborate on this one more? >> >> - keep the same way as in the Kernel > > If kernel does it in the same way it should be also fixed. > > I have been checking yesterday dt binding docs in u-boot and in Linux > and surprisingly they are different. > > ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described > > ti,clk-output-sel is supported but even in code is said that it is > optional property. > > ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth are > not documented in dt binding doc > > ti,sgmii-ref-clock-output-enable is not supported in u-boot but it is in > Linux and we are using this feature. > My understanding is that u-boot goal is to have uboot-dt == kernel-dt and current approach add DT+bindings to the kernel first. So, if you check most of u-boot bindings are missed or obsolete. > Can you please sync it if you want to keep it in the same was as is done > in Linux? So, may be it can be just deleted. > >> - code readability > > I don't think this is really changing code readability. For improving > readability would be the best to move bodies of these ifs to separate > functions and not have dp83867_config() ~140 lines long. It really too minor change to fight for. But if you wish you can update kernel driver as per-above your proposal and then port it in u-boot. -- Best regards, grygorii ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-13 15:49 ` Grygorii Strashko @ 2020-02-13 16:05 ` Michal Simek 2020-02-19 16:36 ` Grygorii Strashko 0 siblings, 1 reply; 9+ messages in thread From: Michal Simek @ 2020-02-13 16:05 UTC (permalink / raw) To: u-boot On 13. 02. 20 16:49, Grygorii Strashko wrote: > > > On 13/02/2020 08:23, Michal Simek wrote: >> On 12. 02. 20 21:24, Grygorii Strashko wrote: >>> >>> >>> On 11/02/2020 10:11, Michal Simek wrote: >>>> On 10. 02. 20 13:07, Grygorii Strashko wrote: >>>>> >>>>> >>>>> On 07/02/2020 13:31, Michal Simek wrote: >>>>>> There is no reason to check sgmii branch again when it is clear that >>>>>> phy >>>>>> interface is rgmii. >>>>>> >>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>>> --- >>>>>> >>>>>> ??? drivers/net/phy/dp83867.c | 4 +--- >>>>>> ??? 1 file changed, 1 insertion(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>>>>> index 4d796e289c45..3178787ff1c7 100644 >>>>>> --- a/drivers/net/phy/dp83867.c >>>>>> +++ b/drivers/net/phy/dp83867.c >>>>>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device >>>>>> *phydev) >>>>>> ??? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>>>>> ????????????????????? DP83867_RGMIIDCTL, delay); >>>>>> -??? } >>>>>> - >>>>>> -??? if (phy_interface_is_sgmii(phydev)) { >>>>>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>>>>> ??????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>>>>> ????????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >>>>>> ?? >>>>> >>>>> ??From one side I have no objections, but from another - I'd prefer to >>>>> keep as is. >>>> >>>> Can you please be elaborate on this one more? >>> >>> - keep the same way as in the Kernel >> >> If kernel does it in the same way it should be also fixed. >> >> I have been checking yesterday dt binding docs in u-boot and in Linux >> and surprisingly they are different. >> >> ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described >> >> ti,clk-output-sel is supported but even in code is said that it is >> optional property. >> >> ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth? are >> not documented in dt binding doc >> >> ti,sgmii-ref-clock-output-enable is not supported in u-boot but it is in >> Linux and we are using this feature. >> > > My understanding is that u-boot goal is to have uboot-dt == kernel-dt > and current > approach add DT+bindings to the kernel first. > > So, if you check most of u-boot bindings are missed or obsolete. > >> Can you please sync it if you want to keep it in the same was as is done >> in Linux? > > So, may be it can be just deleted. I am ok with that. > >> >>> - code readability >> >> I don't think this is really changing code readability. For improving >> readability would be the best to move bodies of these ifs to separate >> functions and not have dp83867_config() ~140 lines long. > > It really too minor change to fight for. But if you wish you can update > kernel > driver as per-above your proposal and then port it in u-boot. I was looking at latest kernel code and it is designed differently there. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/phy/dp83867.c?h=next-20200213#n479 there is if rgmii or sgmii if rgmii io impedance if sgmii in u-boot you have if rgmii if sgmii io impedance I am ok with having this in sync but that's not what we have today. Thanks, Michal ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-13 16:05 ` Michal Simek @ 2020-02-19 16:36 ` Grygorii Strashko 2020-02-20 8:08 ` Michal Simek 0 siblings, 1 reply; 9+ messages in thread From: Grygorii Strashko @ 2020-02-19 16:36 UTC (permalink / raw) To: u-boot On 13/02/2020 18:05, Michal Simek wrote: > On 13. 02. 20 16:49, Grygorii Strashko wrote: >> >> >> On 13/02/2020 08:23, Michal Simek wrote: >>> On 12. 02. 20 21:24, Grygorii Strashko wrote: >>>> >>>> >>>> On 11/02/2020 10:11, Michal Simek wrote: >>>>> On 10. 02. 20 13:07, Grygorii Strashko wrote: >>>>>> >>>>>> >>>>>> On 07/02/2020 13:31, Michal Simek wrote: >>>>>>> There is no reason to check sgmii branch again when it is clear that >>>>>>> phy >>>>>>> interface is rgmii. >>>>>>> >>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>>>> --- >>>>>>> >>>>>>> ??? drivers/net/phy/dp83867.c | 4 +--- >>>>>>> ??? 1 file changed, 1 insertion(+), 3 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>>>>>> index 4d796e289c45..3178787ff1c7 100644 >>>>>>> --- a/drivers/net/phy/dp83867.c >>>>>>> +++ b/drivers/net/phy/dp83867.c >>>>>>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device >>>>>>> *phydev) >>>>>>> ??? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>>>>>> ????????????????????? DP83867_RGMIIDCTL, delay); >>>>>>> -??? } >>>>>>> - >>>>>>> -??? if (phy_interface_is_sgmii(phydev)) { >>>>>>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>>>>>> ??????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>>>>>> ????????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000)); >>>>>>> >>>>>> >>>>>> ??From one side I have no objections, but from another - I'd prefer to >>>>>> keep as is. >>>>> >>>>> Can you please be elaborate on this one more? >>>> >>>> - keep the same way as in the Kernel >>> >>> If kernel does it in the same way it should be also fixed. >>> >>> I have been checking yesterday dt binding docs in u-boot and in Linux >>> and surprisingly they are different. >>> >>> ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described >>> >>> ti,clk-output-sel is supported but even in code is said that it is >>> optional property. >>> >>> ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth? are >>> not documented in dt binding doc >>> >>> ti,sgmii-ref-clock-output-enable is not supported in u-boot but it is in >>> Linux and we are using this feature. >>> >> >> My understanding is that u-boot goal is to have uboot-dt == kernel-dt >> and current >> approach add DT+bindings to the kernel first. >> >> So, if you check most of u-boot bindings are missed or obsolete. >> >>> Can you please sync it if you want to keep it in the same was as is done >>> in Linux? >> >> So, may be it can be just deleted. > > I am ok with that. > >> >>> >>>> - code readability >>> >>> I don't think this is really changing code readability. For improving >>> readability would be the best to move bodies of these ifs to separate >>> functions and not have dp83867_config() ~140 lines long. >> >> It really too minor change to fight for. But if you wish you can update >> kernel >> driver as per-above your proposal and then port it in u-boot. > > I was looking at latest kernel code and it is designed differently there. > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/phy/dp83867.c?h=next-20200213#n479 > > > there is > if rgmii or sgmii > if rgmii > io impedance > if sgmii > > in u-boot you have > if rgmii > if sgmii > io impedance > > I am ok with having this in sync but that's not what we have today. Yes. Unfortunately, The kernel moves forward faster than u-boot and people not always interesting to port their patches in u-boot :( Plus, not all functionality, enabled in the kernel, is really required by u-boot. -- Best regards, grygorii ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used 2020-02-19 16:36 ` Grygorii Strashko @ 2020-02-20 8:08 ` Michal Simek 0 siblings, 0 replies; 9+ messages in thread From: Michal Simek @ 2020-02-20 8:08 UTC (permalink / raw) To: u-boot On 19. 02. 20 17:36, Grygorii Strashko wrote: > > > On 13/02/2020 18:05, Michal Simek wrote: >> On 13. 02. 20 16:49, Grygorii Strashko wrote: >>> >>> >>> On 13/02/2020 08:23, Michal Simek wrote: >>>> On 12. 02. 20 21:24, Grygorii Strashko wrote: >>>>> >>>>> >>>>> On 11/02/2020 10:11, Michal Simek wrote: >>>>>> On 10. 02. 20 13:07, Grygorii Strashko wrote: >>>>>>> >>>>>>> >>>>>>> On 07/02/2020 13:31, Michal Simek wrote: >>>>>>>> There is no reason to check sgmii branch again when it is clear >>>>>>>> that >>>>>>>> phy >>>>>>>> interface is rgmii. >>>>>>>> >>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>>>>> --- >>>>>>>> >>>>>>>> ???? drivers/net/phy/dp83867.c | 4 +--- >>>>>>>> ???? 1 file changed, 1 insertion(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >>>>>>>> index 4d796e289c45..3178787ff1c7 100644 >>>>>>>> --- a/drivers/net/phy/dp83867.c >>>>>>>> +++ b/drivers/net/phy/dp83867.c >>>>>>>> @@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device >>>>>>>> *phydev) >>>>>>>> ???? ????????? phy_write_mmd(phydev, DP83867_DEVADDR, >>>>>>>> ?????????????????????? DP83867_RGMIIDCTL, delay); >>>>>>>> -??? } >>>>>>>> - >>>>>>>> -??? if (phy_interface_is_sgmii(phydev)) { >>>>>>>> +??? } else if (phy_interface_is_sgmii(phydev)) { >>>>>>>> ???????????? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, >>>>>>>> ?????????????????? (BMCR_ANENABLE | BMCR_FULLDPLX | >>>>>>>> BMCR_SPEED1000)); >>>>>>>> ??? >>>>>>> >>>>>>> ???From one side I have no objections, but from another - I'd >>>>>>> prefer to >>>>>>> keep as is. >>>>>> >>>>>> Can you please be elaborate on this one more? >>>>> >>>>> - keep the same way as in the Kernel >>>> >>>> If kernel does it in the same way it should be also fixed. >>>> >>>> I have been checking yesterday dt binding docs in u-boot and in Linux >>>> and surprisingly they are different. >>>> >>>> ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described >>>> >>>> ti,clk-output-sel is supported but even in code is said that it is >>>> optional property. >>>> >>>> ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth? are >>>> not documented in dt binding doc >>>> >>>> ti,sgmii-ref-clock-output-enable is not supported in u-boot but it >>>> is in >>>> Linux and we are using this feature. >>>> >>> >>> My understanding is that u-boot goal is to have uboot-dt == kernel-dt >>> and current >>> approach add DT+bindings to the kernel first. >>> >>> So, if you check most of u-boot bindings are missed or obsolete. >>> >>>> Can you please sync it if you want to keep it in the same was as is >>>> done >>>> in Linux? >>> >>> So, may be it can be just deleted. >> >> I am ok with that. >> >>> >>>> >>>>> - code readability >>>> >>>> I don't think this is really changing code readability. For improving >>>> readability would be the best to move bodies of these ifs to separate >>>> functions and not have dp83867_config() ~140 lines long. >>> >>> It really too minor change to fight for. But if you wish you can update >>> kernel >>> driver as per-above your proposal and then port it in u-boot. >> >> I was looking at latest kernel code and it is designed differently there. >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/phy/dp83867.c?h=next-20200213#n479 >> >> >> >> there is >> if rgmii or sgmii >> if rgmii >> io impedance >> if sgmii >> >> in u-boot you have >> if rgmii >> if sgmii >> io impedance >> >> I am ok with having this in sync but that's not what we have today. > > Yes. Unfortunately, The kernel moves forward faster than u-boot and > people not always > interesting to port their patches in u-boot :( > Plus, not all functionality, enabled in the kernel, is really required > by u-boot. Anyway I have sent the patch for sgmii-ref-clock. If you can review that will be great. And if you want to sync it later I am fine with it. Thanks, Michal ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-02-20 8:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-07 11:31 [PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used Michal Simek 2020-02-10 12:07 ` Grygorii Strashko 2020-02-11 8:11 ` Michal Simek 2020-02-12 20:24 ` Grygorii Strashko 2020-02-13 6:23 ` Michal Simek 2020-02-13 15:49 ` Grygorii Strashko 2020-02-13 16:05 ` Michal Simek 2020-02-19 16:36 ` Grygorii Strashko 2020-02-20 8:08 ` Michal Simek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.