All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Clay McClure <clay@daemons.net>
Cc: Arnd Bergmann <arnd@arndb.de>, kbuild test robot <lkp@intel.com>,
	Russell King <linux@armlinux.org.uk>,
	Tony Lindgren <tony@atomide.com>,
	"David S. Miller" <davem@davemloft.net>,
	Murali Karicheri <m-karicheri2@ti.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Andrew Jeffery <andrew@aj.id.au>,
	Santosh Shilimkar <santosh.shilimkar@oracle.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>,
	Richard Cochran <richardcochran@gmail.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
	<netdev@vger.kernel.org>
Subject: Re: [PATCH v2] net: ethernet: ti: Remove TI_CPTS_MOD workaround
Date: Wed, 6 May 2020 23:56:35 +0300	[thread overview]
Message-ID: <1654101f-9dd7-2e10-7344-0d08e32bc309@ti.com> (raw)
In-Reply-To: <20200506065105.GA3226@arctic-shiba-lx>



On 06/05/2020 09:51, Clay McClure wrote:
> On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote:
> 
>> 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.
> 
>> 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

WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Clay McClure <clay@daemons.net>
Cc: Tony Lindgren <tony@atomide.com>,
	kbuild test robot <lkp@intel.com>,
	Russell King <linux@armlinux.org.uk>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Murali Karicheri <m-karicheri2@ti.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Kees Cook <keescook@chromium.org>, Arnd Bergmann <arnd@arndb.de>,
	Richard Cochran <richardcochran@gmail.com>,
	Santosh Shilimkar <santosh.shilimkar@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>,
	Andrew Jeffery <andrew@aj.id.au>,
	Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	linux-kernel@vger.kernel.org,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2] net: ethernet: ti: Remove TI_CPTS_MOD workaround
Date: Wed, 6 May 2020 23:56:35 +0300	[thread overview]
Message-ID: <1654101f-9dd7-2e10-7344-0d08e32bc309@ti.com> (raw)
In-Reply-To: <20200506065105.GA3226@arctic-shiba-lx>



On 06/05/2020 09:51, Clay McClure wrote:
> On Tue, May 05, 2020 at 10:41:26AM +0300, Grygorii Strashko wrote:
> 
>> 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.
> 
>> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-05-06 20:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-02 23:39 [PATCH] net: ethernet: ti: Remove TI_CPTS_MOD workaround Clay McClure
2020-05-02 23:39 ` Clay McClure
2020-05-04 15:09 ` Tony Lindgren
2020-05-04 15:09   ` Tony Lindgren
2020-05-04 15:16 ` Arnd Bergmann
2020-05-04 15:16   ` Arnd Bergmann
2020-05-04 16:57   ` [PATCH v2] " Clay McClure
2020-05-04 16:57     ` Clay McClure
2020-05-05  7:41     ` Grygorii Strashko
2020-05-05  7:41       ` Grygorii Strashko
2020-05-06  6:51       ` Clay McClure
2020-05-06  6:51         ` Clay McClure
2020-05-06 20:56         ` Grygorii Strashko [this message]
2020-05-06 20:56           ` Grygorii Strashko
2020-05-07 21:37           ` Arnd Bergmann
2020-05-07 21:37             ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1654101f-9dd7-2e10-7344-0d08e32bc309@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=andrew@aj.id.au \
    --cc=arnd@arndb.de \
    --cc=brouer@redhat.com \
    --cc=clay@daemons.net \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=ivan.khoronzhuk@linaro.org \
    --cc=keescook@chromium.org \
    --cc=krzk@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=lkp@intel.com \
    --cc=m-karicheri2@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=pankaj.laxminarayan.bharadiya@intel.com \
    --cc=richardcochran@gmail.com \
    --cc=santosh.shilimkar@oracle.com \
    --cc=tglx@linutronix.de \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.