| Message ID | 20220501124131.30614-2-josua@solid-run.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | mx6cuboxi: add eth support for SoMs revision 1.9+ | expand |
Hi Josua, On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote: > The adin_get_phy_mode_override function does not compile, because it > is > missing both declaration and implementation of > phy_get_interface_by_name. > > Remove the whole function for now, since the missing implementation > is > not included in mainline Linux - and thus can not be copied. > > Signed-off-by: Josua Mayer <josua@solid-run.com> > --- > drivers/net/phy/adin.c | 34 ---------------------------------- > 1 file changed, 34 deletions(-) > > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > index cff841ab3d..2433e76fea 100644 > --- a/drivers/net/phy/adin.c > +++ b/drivers/net/phy/adin.c > @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device > *phydev, > return rc; > } > > -/** > - * adin_get_phy_mode_override - Get phy-mode override for adin PHY > - * > - * The function gets phy-mode string from property 'adi,phy-mode- > override' > - * and return its index in phy_interface_strings table, or -1 in > error case. > - */ > -int adin_get_phy_mode_override(struct phy_device *phydev) > -{ > - ofnode node = phy_get_ofnode(phydev); > - const char *phy_mode_override; > - const char *prop_phy_mode_override = "adi,phy-mode-override"; > - int override_interface; > - > - phy_mode_override = ofnode_read_string(node, > prop_phy_mode_override); > - if (!phy_mode_override) > - return -ENODEV; > - > - debug("%s: %s = '%s'\n", > - __func__, prop_phy_mode_override, phy_mode_override); > - > - override_interface = > phy_get_interface_by_name(phy_mode_override); > - > - if (override_interface < 0) > - printf("%s: %s = '%s' is not valid\n", > - __func__, prop_phy_mode_override, > phy_mode_override); > - > - return override_interface; > -} > - > static u16 adin_ext_read(struct phy_device *phydev, const u32 > regnum) > { > u16 val; > @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct > phy_device *phydev) > { > u16 reg_val; > u32 val; > - int phy_mode_override = adin_get_phy_mode_override(phydev); > - > - if (phy_mode_override >= 0) { > - phydev->interface = (phy_interface_t) > phy_mode_override; > - } > > reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); > The patch that introduced adin_get_phy_mode_override was tested against the U-Boot master branch. Unfortunately, phy_get_interface_by_name was removed just a few days before by: https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8b8b5 Can we fix adin_get_phy_mode_override instead of removing it? We can implement part of ofnode_read_phy_mode. This needs to be tested, but for example: diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..6a05b9fd05 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device *phydev, * The function gets phy-mode string from property 'adi,phy-mode- override' * and return its index in phy_interface_strings table, or -1 in error case. */ -int adin_get_phy_mode_override(struct phy_device *phydev) +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) { ofnode node = phy_get_ofnode(phydev); const char *phy_mode_override; const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface; + int i; phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); + if (!phy_mode_override) - return -ENODEV; + return PHY_INTERFACE_MODE_NA; debug("%s: %s = '%s'\n", __func__, prop_phy_mode_override, phy_mode_override); - override_interface = phy_get_interface_by_name(phy_mode_override); + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) + if (!strcmp(phy_mode_override, phy_interface_strings[i])) + return i; - if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); + debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override); - return override_interface; + return PHY_INTERFACE_MODE_NA; } static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); + phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev); - if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { + phydev->interface = phy_mode_override; } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); Thanks, Nate
Hi Nate, Am 02.05.22 um 16:25 schrieb Nate Drude: > Hi Josua, > > On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote: >> The adin_get_phy_mode_override function does not compile, because it >> is >> missing both declaration and implementation of >> phy_get_interface_by_name. >> >> Remove the whole function for now, since the missing implementation >> is >> not included in mainline Linux - and thus can not be copied. >> >> Signed-off-by: Josua Mayer <josua@solid-run.com> >> --- >> drivers/net/phy/adin.c | 34 ---------------------------------- >> 1 file changed, 34 deletions(-) >> >> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c >> index cff841ab3d..2433e76fea 100644 >> --- a/drivers/net/phy/adin.c >> +++ b/drivers/net/phy/adin.c >> @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device >> *phydev, >> return rc; >> } >> >> -/** >> - * adin_get_phy_mode_override - Get phy-mode override for adin PHY >> - * >> - * The function gets phy-mode string from property 'adi,phy-mode- >> override' >> - * and return its index in phy_interface_strings table, or -1 in >> error case. >> - */ >> -int adin_get_phy_mode_override(struct phy_device *phydev) >> -{ >> - ofnode node = phy_get_ofnode(phydev); >> - const char *phy_mode_override; >> - const char *prop_phy_mode_override = "adi,phy-mode-override"; >> - int override_interface; >> - >> - phy_mode_override = ofnode_read_string(node, >> prop_phy_mode_override); >> - if (!phy_mode_override) >> - return -ENODEV; >> - >> - debug("%s: %s = '%s'\n", >> - __func__, prop_phy_mode_override, phy_mode_override); >> - >> - override_interface = >> phy_get_interface_by_name(phy_mode_override); >> - >> - if (override_interface < 0) >> - printf("%s: %s = '%s' is not valid\n", >> - __func__, prop_phy_mode_override, >> phy_mode_override); >> - >> - return override_interface; >> -} >> - >> static u16 adin_ext_read(struct phy_device *phydev, const u32 >> regnum) >> { >> u16 val; >> @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct >> phy_device *phydev) >> { >> u16 reg_val; >> u32 val; >> - int phy_mode_override = adin_get_phy_mode_override(phydev); >> - >> - if (phy_mode_override >= 0) { >> - phydev->interface = (phy_interface_t) >> phy_mode_override; >> - } >> >> reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); >> > The patch that introduced adin_get_phy_mode_override was tested against > the U-Boot master branch. Unfortunately, phy_get_interface_by_name was > removed just a few days before by: > https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8b8b5 :( > Can we fix adin_get_phy_mode_override instead of removing it? I will take a look in the coming days and see if I understand how to fix this. I don't really mind if we fix it, or remove it - removing was the faster way getting my patch-set out of course ... > We can > implement part of ofnode_read_phy_mode. This needs to be tested, but > for example: > > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > index cff841ab3d..6a05b9fd05 100644 > --- a/drivers/net/phy/adin.c > +++ b/drivers/net/phy/adin.c > @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device > *phydev, > * The function gets phy-mode string from property 'adi,phy-mode- > override' > * and return its index in phy_interface_strings table, or -1 in error > case. > */ > -int adin_get_phy_mode_override(struct phy_device *phydev) > +phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev) > { > ofnode node = phy_get_ofnode(phydev); > const char *phy_mode_override; > const char *prop_phy_mode_override = "adi,phy-mode-override"; > - int override_interface; > + int i; > > phy_mode_override = ofnode_read_string(node, > prop_phy_mode_override); > + > if (!phy_mode_override) > - return -ENODEV; > + return PHY_INTERFACE_MODE_NA; > > debug("%s: %s = '%s'\n", > __func__, prop_phy_mode_override, phy_mode_override); > > - override_interface = > phy_get_interface_by_name(phy_mode_override); > + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) > + if (!strcmp(phy_mode_override, > phy_interface_strings[i])) > + return i; > > - if (override_interface < 0) > - printf("%s: %s = '%s' is not valid\n", > - __func__, prop_phy_mode_override, > phy_mode_override); > + debug("%s: Invalid PHY interface '%s'\n", __func__, > phy_mode_override); > > - return override_interface; > + return PHY_INTERFACE_MODE_NA; > } > > static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) > @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct > phy_device *phydev) > { > u16 reg_val; > u32 val; > - int phy_mode_override = adin_get_phy_mode_override(phydev); > + phy_interface_t phy_mode_override = > adin_get_phy_mode_override(phydev); > > - if (phy_mode_override >= 0) { > - phydev->interface = (phy_interface_t) > phy_mode_override; > + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { > + phydev->interface = phy_mode_override; > } > > reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); > > Thanks, > Nate
Hi Josua, On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote: > Hi Nate, > > Am 02.05.22 um 16:25 schrieb Nate Drude: > > Hi Josua, > > > > On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote: > > > The adin_get_phy_mode_override function does not compile, because > > > it > > > is > > > missing both declaration and implementation of > > > phy_get_interface_by_name. > > > > > > Remove the whole function for now, since the missing > > > implementation > > > is > > > not included in mainline Linux - and thus can not be copied. > > > > > > Signed-off-by: Josua Mayer <josua@solid-run.com> > > > --- > > > drivers/net/phy/adin.c | 34 ---------------------------------- > > > 1 file changed, 34 deletions(-) > > > > > > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > > > index cff841ab3d..2433e76fea 100644 > > > --- a/drivers/net/phy/adin.c > > > +++ b/drivers/net/phy/adin.c > > > @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct > > > phy_device > > > *phydev, > > > return rc; > > > } > > > > > > -/** > > > - * adin_get_phy_mode_override - Get phy-mode override for adin > > > PHY > > > - * > > > - * The function gets phy-mode string from property 'adi,phy- > > > mode- > > > override' > > > - * and return its index in phy_interface_strings table, or -1 in > > > error case. > > > - */ > > > -int adin_get_phy_mode_override(struct phy_device *phydev) > > > -{ > > > - ofnode node = phy_get_ofnode(phydev); > > > - const char *phy_mode_override; > > > - const char *prop_phy_mode_override = "adi,phy-mode- > > > override"; > > > - int override_interface; > > > - > > > - phy_mode_override = ofnode_read_string(node, > > > prop_phy_mode_override); > > > - if (!phy_mode_override) > > > - return -ENODEV; > > > - > > > - debug("%s: %s = '%s'\n", > > > - __func__, prop_phy_mode_override, > > > phy_mode_override); > > > - > > > - override_interface = > > > phy_get_interface_by_name(phy_mode_override); > > > - > > > - if (override_interface < 0) > > > - printf("%s: %s = '%s' is not valid\n", > > > - __func__, prop_phy_mode_override, > > > phy_mode_override); > > > - > > > - return override_interface; > > > -} > > > - > > > static u16 adin_ext_read(struct phy_device *phydev, const u32 > > > regnum) > > > { > > > u16 val; > > > @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct > > > phy_device *phydev) > > > { > > > u16 reg_val; > > > u32 val; > > > - int phy_mode_override = > > > adin_get_phy_mode_override(phydev); > > > - > > > - if (phy_mode_override >= 0) { > > > - phydev->interface = (phy_interface_t) > > > phy_mode_override; > > > - } > > > > > > reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); > > > > > The patch that introduced adin_get_phy_mode_override was tested > > against > > the U-Boot master branch. Unfortunately, phy_get_interface_by_name > > was > > removed just a few days before by: > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&reserved=0 > :( > > Can we fix adin_get_phy_mode_override instead of removing it? > I will take a look in the coming days and see if I understand how to > fix > this. > I don't really mind if we fix it, or remove it - removing was the > faster If you prefer, I can submit a separate patch to fix it. I'll wait to hear from you to avoid duplicated work. > way getting my patch-set out of course ... > > We can > > implement part of ofnode_read_phy_mode. This needs to be tested, > > but > > for example: > > > > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > > index cff841ab3d..6a05b9fd05 100644 > > --- a/drivers/net/phy/adin.c > > +++ b/drivers/net/phy/adin.c > > @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct > > phy_device > > *phydev, > > * The function gets phy-mode string from property 'adi,phy-mode- > > override' > > * and return its index in phy_interface_strings table, or -1 in > > error > > case. > > */ > > -int adin_get_phy_mode_override(struct phy_device *phydev) > > +phy_interface_t adin_get_phy_mode_override(struct phy_device > > *phydev) > > { > > ofnode node = phy_get_ofnode(phydev); > > const char *phy_mode_override; > > const char *prop_phy_mode_override = "adi,phy-mode- > > override"; > > - int override_interface; > > + int i; > > > > phy_mode_override = ofnode_read_string(node, > > prop_phy_mode_override); > > + > > if (!phy_mode_override) > > - return -ENODEV; > > + return PHY_INTERFACE_MODE_NA; > > > > debug("%s: %s = '%s'\n", > > __func__, prop_phy_mode_override, > > phy_mode_override); > > > > - override_interface = > > phy_get_interface_by_name(phy_mode_override); > > + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) > > + if (!strcmp(phy_mode_override, > > phy_interface_strings[i])) > > + return i; > > > > - if (override_interface < 0) > > - printf("%s: %s = '%s' is not valid\n", > > - __func__, prop_phy_mode_override, > > phy_mode_override); > > + debug("%s: Invalid PHY interface '%s'\n", __func__, > > phy_mode_override); > > > > - return override_interface; > > + return PHY_INTERFACE_MODE_NA; > > } > > > > static u16 adin_ext_read(struct phy_device *phydev, const u32 > > regnum) > > @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct > > phy_device *phydev) > > { > > u16 reg_val; > > u32 val; > > - int phy_mode_override = adin_get_phy_mode_override(phydev); > > + phy_interface_t phy_mode_override = > > adin_get_phy_mode_override(phydev); > > > > - if (phy_mode_override >= 0) { > > - phydev->interface = (phy_interface_t) > > phy_mode_override; > > + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { > > + phydev->interface = phy_mode_override; > > } > > > > reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); > > > > Thanks, > > Nate Thanks, Nate
\o/ Am 04.05.22 um 15:46 schrieb Nate Drude: > Hi Josua, > > On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote: >> Hi Nate, >> >> Am 02.05.22 um 16:25 schrieb Nate Drude: >>> Hi Josua, >>> >>> On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote: >>>> The adin_get_phy_mode_override function does not compile, because >>>> it >>>> is >>>> missing both declaration and implementation of >>>> phy_get_interface_by_name. >>>> >>>> Remove the whole function for now, since the missing >>>> implementation >>>> is >>>> not included in mainline Linux - and thus can not be copied. >>>> >>>> Signed-off-by: Josua Mayer <josua@solid-run.com> >>>> --- >>>> drivers/net/phy/adin.c | 34 ---------------------------------- >>>> 1 file changed, 34 deletions(-) >>>> >>>> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c >>>> index cff841ab3d..2433e76fea 100644 >>>> --- a/drivers/net/phy/adin.c >>>> +++ b/drivers/net/phy/adin.c >>>> @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct >>>> phy_device >>>> *phydev, >>>> return rc; >>>> } >>>> >>>> -/** >>>> - * adin_get_phy_mode_override - Get phy-mode override for adin >>>> PHY >>>> - * >>>> - * The function gets phy-mode string from property 'adi,phy- >>>> mode- >>>> override' >>>> - * and return its index in phy_interface_strings table, or -1 in >>>> error case. >>>> - */ >>>> -int adin_get_phy_mode_override(struct phy_device *phydev) >>>> -{ >>>> - ofnode node = phy_get_ofnode(phydev); >>>> - const char *phy_mode_override; >>>> - const char *prop_phy_mode_override = "adi,phy-mode- >>>> override"; >>>> - int override_interface; >>>> - >>>> - phy_mode_override = ofnode_read_string(node, >>>> prop_phy_mode_override); >>>> - if (!phy_mode_override) >>>> - return -ENODEV; >>>> - >>>> - debug("%s: %s = '%s'\n", >>>> - __func__, prop_phy_mode_override, >>>> phy_mode_override); >>>> - >>>> - override_interface = >>>> phy_get_interface_by_name(phy_mode_override); >>>> - >>>> - if (override_interface < 0) >>>> - printf("%s: %s = '%s' is not valid\n", >>>> - __func__, prop_phy_mode_override, >>>> phy_mode_override); >>>> - >>>> - return override_interface; >>>> -} >>>> - >>>> static u16 adin_ext_read(struct phy_device *phydev, const u32 >>>> regnum) >>>> { >>>> u16 val; >>>> @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct >>>> phy_device *phydev) >>>> { >>>> u16 reg_val; >>>> u32 val; >>>> - int phy_mode_override = >>>> adin_get_phy_mode_override(phydev); >>>> - >>>> - if (phy_mode_override >= 0) { >>>> - phydev->interface = (phy_interface_t) >>>> phy_mode_override; >>>> - } >>>> >>>> reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); >>>> >>> The patch that introduced adin_get_phy_mode_override was tested >>> against >>> the U-Boot master branch. Unfortunately, phy_get_interface_by_name >>> was >>> removed just a few days before by: >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&reserved=0 >> :( >>> Can we fix adin_get_phy_mode_override instead of removing it? >> I will take a look in the coming days and see if I understand how to >> fix >> this. >> I don't really mind if we fix it, or remove it - removing was the >> faster > If you prefer, I can submit a separate patch to fix it. I'll wait to > hear from you to avoid duplicated work. I won't be able to look at this before Sunday, so feel free to work on it. >> way getting my patch-set out of course ... >>> We can >>> implement part of ofnode_read_phy_mode. This needs to be tested, >>> but >>> for example: >>> >>> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c >>> index cff841ab3d..6a05b9fd05 100644 >>> --- a/drivers/net/phy/adin.c >>> +++ b/drivers/net/phy/adin.c >>> @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct >>> phy_device >>> *phydev, >>> * The function gets phy-mode string from property 'adi,phy-mode- >>> override' >>> * and return its index in phy_interface_strings table, or -1 in >>> error >>> case. >>> */ >>> -int adin_get_phy_mode_override(struct phy_device *phydev) >>> +phy_interface_t adin_get_phy_mode_override(struct phy_device >>> *phydev) >>> { >>> ofnode node = phy_get_ofnode(phydev); >>> const char *phy_mode_override; >>> const char *prop_phy_mode_override = "adi,phy-mode- >>> override"; >>> - int override_interface; >>> + int i; >>> >>> phy_mode_override = ofnode_read_string(node, >>> prop_phy_mode_override); >>> + >>> if (!phy_mode_override) >>> - return -ENODEV; >>> + return PHY_INTERFACE_MODE_NA; >>> >>> debug("%s: %s = '%s'\n", >>> __func__, prop_phy_mode_override, >>> phy_mode_override); >>> >>> - override_interface = >>> phy_get_interface_by_name(phy_mode_override); >>> + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) >>> + if (!strcmp(phy_mode_override, >>> phy_interface_strings[i])) >>> + return i; >>> >>> - if (override_interface < 0) >>> - printf("%s: %s = '%s' is not valid\n", >>> - __func__, prop_phy_mode_override, >>> phy_mode_override); >>> + debug("%s: Invalid PHY interface '%s'\n", __func__, >>> phy_mode_override); >>> >>> - return override_interface; >>> + return PHY_INTERFACE_MODE_NA; >>> } >>> >>> static u16 adin_ext_read(struct phy_device *phydev, const u32 >>> regnum) >>> @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct >>> phy_device *phydev) >>> { >>> u16 reg_val; >>> u32 val; >>> - int phy_mode_override = adin_get_phy_mode_override(phydev); >>> + phy_interface_t phy_mode_override = >>> adin_get_phy_mode_override(phydev); >>> >>> - if (phy_mode_override >= 0) { >>> - phydev->interface = (phy_interface_t) >>> phy_mode_override; >>> + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { >>> + phydev->interface = phy_mode_override; >>> } >>> >>> reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); >>> >>> Thanks, >>> Nate > Thanks, > Nate >
Hi Nate, Maintainers, This patch is now superseded by Nate's [PATCH] phy: adin: fix broken support for adi,phy-mode-override submitted yesterday. The remainder of this patchset cleanly applies on top of it though, so I won't roll v2. Let me know if there is any further feedback on this series, or if it can be applied. Am 05.05.22 um 18:30 schrieb Josua Mayer: > \o/ > > Am 04.05.22 um 15:46 schrieb Nate Drude: >> Hi Josua, >> >> On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote: >>> Hi Nate, >>> >>> Am 02.05.22 um 16:25 schrieb Nate Drude: >>>> Hi Josua, >>>> >>>> On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote: >>>>> The adin_get_phy_mode_override function does not compile, because >>>>> it >>>>> is >>>>> missing both declaration and implementation of >>>>> phy_get_interface_by_name. >>>>> >>>>> Remove the whole function for now, since the missing >>>>> implementation >>>>> is >>>>> not included in mainline Linux - and thus can not be copied. >>>>> >>>>> Signed-off-by: Josua Mayer <josua@solid-run.com> >>>>> --- >>>>> drivers/net/phy/adin.c | 34 ---------------------------------- >>>>> 1 file changed, 34 deletions(-) >>>>> >>>>> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c >>>>> index cff841ab3d..2433e76fea 100644 >>>>> --- a/drivers/net/phy/adin.c >>>>> +++ b/drivers/net/phy/adin.c >>>>> @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct >>>>> phy_device >>>>> *phydev, >>>>> return rc; >>>>> } >>>>> -/** >>>>> - * adin_get_phy_mode_override - Get phy-mode override for adin >>>>> PHY >>>>> - * >>>>> - * The function gets phy-mode string from property 'adi,phy- >>>>> mode- >>>>> override' >>>>> - * and return its index in phy_interface_strings table, or -1 in >>>>> error case. >>>>> - */ >>>>> -int adin_get_phy_mode_override(struct phy_device *phydev) >>>>> -{ >>>>> - ofnode node = phy_get_ofnode(phydev); >>>>> - const char *phy_mode_override; >>>>> - const char *prop_phy_mode_override = "adi,phy-mode- >>>>> override"; >>>>> - int override_interface; >>>>> - >>>>> - phy_mode_override = ofnode_read_string(node, >>>>> prop_phy_mode_override); >>>>> - if (!phy_mode_override) >>>>> - return -ENODEV; >>>>> - >>>>> - debug("%s: %s = '%s'\n", >>>>> - __func__, prop_phy_mode_override, >>>>> phy_mode_override); >>>>> - >>>>> - override_interface = >>>>> phy_get_interface_by_name(phy_mode_override); >>>>> - >>>>> - if (override_interface < 0) >>>>> - printf("%s: %s = '%s' is not valid\n", >>>>> - __func__, prop_phy_mode_override, >>>>> phy_mode_override); >>>>> - >>>>> - return override_interface; >>>>> -} >>>>> - >>>>> static u16 adin_ext_read(struct phy_device *phydev, const u32 >>>>> regnum) >>>>> { >>>>> u16 val; >>>>> @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct >>>>> phy_device *phydev) >>>>> { >>>>> u16 reg_val; >>>>> u32 val; >>>>> - int phy_mode_override = >>>>> adin_get_phy_mode_override(phydev); >>>>> - >>>>> - if (phy_mode_override >= 0) { >>>>> - phydev->interface = (phy_interface_t) >>>>> phy_mode_override; >>>>> - } >>>>> reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); >>>> The patch that introduced adin_get_phy_mode_override was tested >>>> against >>>> the U-Boot master branch. Unfortunately, phy_get_interface_by_name >>>> was >>>> removed just a few days before by: >>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5&data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3D&reserved=0 >>>> >>> :( >>>> Can we fix adin_get_phy_mode_override instead of removing it? >>> I will take a look in the coming days and see if I understand how to >>> fix >>> this. >>> I don't really mind if we fix it, or remove it - removing was the >>> faster >> If you prefer, I can submit a separate patch to fix it. I'll wait to >> hear from you to avoid duplicated work. > I won't be able to look at this before Sunday, so feel free to work on > it. >>> way getting my patch-set out of course ... >>>> We can >>>> implement part of ofnode_read_phy_mode. This needs to be tested, >>>> but >>>> for example: >>>> >>>> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c >>>> index cff841ab3d..6a05b9fd05 100644 >>>> --- a/drivers/net/phy/adin.c >>>> +++ b/drivers/net/phy/adin.c >>>> @@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct >>>> phy_device >>>> *phydev, >>>> * The function gets phy-mode string from property 'adi,phy-mode- >>>> override' >>>> * and return its index in phy_interface_strings table, or -1 in >>>> error >>>> case. >>>> */ >>>> -int adin_get_phy_mode_override(struct phy_device *phydev) >>>> +phy_interface_t adin_get_phy_mode_override(struct phy_device >>>> *phydev) >>>> { >>>> ofnode node = phy_get_ofnode(phydev); >>>> const char *phy_mode_override; >>>> const char *prop_phy_mode_override = "adi,phy-mode- >>>> override"; >>>> - int override_interface; >>>> + int i; >>>> phy_mode_override = ofnode_read_string(node, >>>> prop_phy_mode_override); >>>> + >>>> if (!phy_mode_override) >>>> - return -ENODEV; >>>> + return PHY_INTERFACE_MODE_NA; >>>> debug("%s: %s = '%s'\n", >>>> __func__, prop_phy_mode_override, >>>> phy_mode_override); >>>> - override_interface = >>>> phy_get_interface_by_name(phy_mode_override); >>>> + for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++) >>>> + if (!strcmp(phy_mode_override, >>>> phy_interface_strings[i])) >>>> + return i; >>>> - if (override_interface < 0) >>>> - printf("%s: %s = '%s' is not valid\n", >>>> - __func__, prop_phy_mode_override, >>>> phy_mode_override); >>>> + debug("%s: Invalid PHY interface '%s'\n", __func__, >>>> phy_mode_override); >>>> - return override_interface; >>>> + return PHY_INTERFACE_MODE_NA; >>>> } >>>> static u16 adin_ext_read(struct phy_device *phydev, const u32 >>>> regnum) >>>> @@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct >>>> phy_device *phydev) >>>> { >>>> u16 reg_val; >>>> u32 val; >>>> - int phy_mode_override = adin_get_phy_mode_override(phydev); >>>> + phy_interface_t phy_mode_override = >>>> adin_get_phy_mode_override(phydev); >>>> - if (phy_mode_override >= 0) { >>>> - phydev->interface = (phy_interface_t) >>>> phy_mode_override; >>>> + if (phy_mode_override != PHY_INTERFACE_MODE_NA) { >>>> + phydev->interface = phy_mode_override; >>>> } >>>> reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG); >>>> >>>> Thanks, >>>> Nate >> Thanks, >> Nate >> - Josua Mayer
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index cff841ab3d..2433e76fea 100644 --- a/drivers/net/phy/adin.c +++ b/drivers/net/phy/adin.c @@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev, return rc; } -/** - * adin_get_phy_mode_override - Get phy-mode override for adin PHY - * - * The function gets phy-mode string from property 'adi,phy-mode-override' - * and return its index in phy_interface_strings table, or -1 in error case. - */ -int adin_get_phy_mode_override(struct phy_device *phydev) -{ - ofnode node = phy_get_ofnode(phydev); - const char *phy_mode_override; - const char *prop_phy_mode_override = "adi,phy-mode-override"; - int override_interface; - - phy_mode_override = ofnode_read_string(node, prop_phy_mode_override); - if (!phy_mode_override) - return -ENODEV; - - debug("%s: %s = '%s'\n", - __func__, prop_phy_mode_override, phy_mode_override); - - override_interface = phy_get_interface_by_name(phy_mode_override); - - if (override_interface < 0) - printf("%s: %s = '%s' is not valid\n", - __func__, prop_phy_mode_override, phy_mode_override); - - return override_interface; -} - static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum) { u16 val; @@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device *phydev) { u16 reg_val; u32 val; - int phy_mode_override = adin_get_phy_mode_override(phydev); - - if (phy_mode_override >= 0) { - phydev->interface = (phy_interface_t) phy_mode_override; - } reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
The adin_get_phy_mode_override function does not compile, because it is missing both declaration and implementation of phy_get_interface_by_name. Remove the whole function for now, since the missing implementation is not included in mainline Linux - and thus can not be copied. Signed-off-by: Josua Mayer <josua@solid-run.com> --- drivers/net/phy/adin.c | 34 ---------------------------------- 1 file changed, 34 deletions(-)