[PATCH 05/13] net: fm: call dtsec_init_phy() only when it is defined

[Jerome Forissier]

On 8/29/24 16:04, Simon Glass wrote:
> Hi Jerome,
> 
> On Wed, 28 Aug 2024 at 06:11, Jerome Forissier
> <jerome.forissier at linaro.org> wrote:
>>
>> dtsec_init_phy() is defined only with MII so add the proper conditional
>> in the caller code.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> ---
>>  drivers/net/fm/eth.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
>> index 19f3f0fef0..e4ec769693 100644
>> --- a/drivers/net/fm/eth.c
>> +++ b/drivers/net/fm/eth.c
>> @@ -701,8 +701,10 @@ static int init_phy(struct fm_eth *fm_eth)
>>                 supported |= SUPPORTED_2500baseX_Full;
>>  #endif
>>
>> +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) && !defined(BITBANGMII)
>>         if (fm_eth->type == FM_ETH_1G_E)
>>                 dtsec_init_phy(fm_eth);
>> +#endif
> 
> We really want to avoid #ifdefs in the code as they create different
> build paths. Can you use if (IS_ENABLED) ?

Sure.

> Also BITBANGMII seems liike
> it needs a CONFIG_ prefix.

I agree. I have copied the line verbatim from line 29 because the condition
needs to be the same. And I suspect there is another issue with the operator
precedence. Do we want "MII || (CMD_MII && !BITBANGMII)" or
do we rather want "(MII || CMD_MII) && !BITBANGMII"? I guess the latter.

So how about:

diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c
index 19f3f0fef0..22025b6a27 100644
--- a/drivers/net/fm/eth.c
+++ b/drivers/net/fm/eth.c
@@ -26,7 +26,8 @@
 
 #include "fm.h"
 
-#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) && !defined(BITBANGMII)
+#if ((defined(CONFIG_MII) || defined(CONFIG_CMD_MII)) && \
+     !defined(CONFIG_BITBANGMII))
 
 #define TBIANA_SETTINGS (TBIANA_ASYMMETRIC_PAUSE | TBIANA_SYMMETRIC_PAUSE | \
 			 TBIANA_FULL_DUPLEX)
@@ -701,8 +702,11 @@ static int init_phy(struct fm_eth *fm_eth)
 		supported |= SUPPORTED_2500baseX_Full;
 #endif
 
-	if (fm_eth->type == FM_ETH_1G_E)
-		dtsec_init_phy(fm_eth);
+	if ((IS_ENABLED(CONFIG_MII) || IS_ENABLED(CONFIG_CMD_MII)) &&
+	    !IS_ENABLED(CONFIG_BITBANGMII)) {
+		if (fm_eth->type == FM_ETH_1G_E)
+			dtsec_init_phy(fm_eth);
+	}
 
 #ifdef CONFIG_PHYLIB
 #ifdef CONFIG_DM_MDIO


???

Thanks,
-- 
Jerome