On 06/05/2020 09:51, Clay McClure wrote:

On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote:

quoted

It's better if you send v2 not as reply to v1.

Noted, thank you, and thank you for taking the time to review my patch.

quoted

just to clarify. After these two patches
  - the PTP_1588_CLOCK can still be set to "M"
  - which will cause TI_CPTS to be "M",
  - but TI_CPSW will still be "Y".

and all above will build and produce built-in CPSW without CPTS support
and cpts.ko which is loadable, but not functional.

Sorry, I'm a little bit lost regarding the target you'are trying to achieve.
At least previously "imply PTP_1588_CLOCK" allowed to select properly PTP_1588_CLOCK
without modifying every defconfig.

Well, I just wanted to squelch a WARN_ON(). As Arnd pointed out in [1],
code that uses the stubbed cpts functions is supposed to gracefully
handle receiving a null pointer. Splatting a warning is not graceful,
and that's what I was trying to fix.

But your question in [2] prompted me to consider whether it should be
possible to build TI_CPTS without PTP_1588_CLOCK at all. I think the
answer is no, so I tried to fix it, but you're right, it's still
possible to end up with a nonfunctional module after my patch.

If you prefer to revert, that's fine with me. Should I post a patch, or
just ask David to revert?

Ok. After some thinking and hence you commit b6d49cab44b5 ("net: Make PTP-specific drivers depend on PTP_1588_CLOCK")
seems was merged in net (not net-next)
1) for-net: defconfig changes can be separated to fix build fail, but add change for multi_v7_defconfig
2) for-net-next: rest of changes plus below diff on top of it

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index f3f8bb724294..62f809b67469 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig

@@ -49,6 +49,7 @@ config TI_CPSW_PHY_SEL
  config TI_CPSW
         tristate "TI CPSW Switch Support"
         depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
+       depends on TI_CPTS || !TI_CPTS
         select TI_DAVINCI_MDIO
         select MFD_SYSCON
         select PAGE_POOL

@@ -64,6 +65,7 @@ config TI_CPSW_SWITCHDEV
         tristate "TI CPSW Switch Support with switchdev"
         depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
         depends on NET_SWITCHDEV
+       depends on TI_CPTS || !TI_CPTS
         select PAGE_POOL
         select TI_DAVINCI_MDIO
         select MFD_SYSCON

@@ -78,11 +80,9 @@ config TI_CPSW_SWITCHDEV
  
  config TI_CPTS
         tristate "TI Common Platform Time Sync (CPTS) Support"
-       depends on TI_CPSW || TI_KEYSTONE_NETCP || TI_CPSW_SWITCHDEV || COMPILE_TEST
+       depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || COMPILE_TEST
         depends on COMMON_CLK
         depends on PTP_1588_CLOCK
-       default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y
-       default m
         ---help---
           This driver supports the Common Platform Time Sync unit of
           the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem.

@@ -109,6 +109,7 @@ config TI_KEYSTONE_NETCP
         select TI_DAVINCI_MDIO
         depends on OF
         depends on KEYSTONE_NAVIGATOR_DMA && KEYSTONE_NAVIGATOR_QMSS
+       depends on TI_CPTS || !TI_CPTS
         ---help---
           This driver supports TI's Keystone NETCP Core.

It should properly resolve "M" vs "Y" dependencies between
  PTP_1588_CLOCK->TI_CPTS->TI_CPSW

On thing, TI_CPTS can be selected without TI_CPSW, but it's probably ok.

-- 
Best regards,
grygorii