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
next prev parent 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: linkBe 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.