From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755870AbdCTQrV (ORCPT ); Mon, 20 Mar 2017 12:47:21 -0400 Received: from mail-yw0-f178.google.com ([209.85.161.178]:35119 "EHLO mail-yw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753328AbdCTQrQ (ORCPT ); Mon, 20 Mar 2017 12:47:16 -0400 MIME-Version: 1.0 In-Reply-To: <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> References: <1488520809-31670-4-git-send-email-leo.yan@linaro.org> <011fdac0-59bf-b539-2de3-0b59a41bc922@arm.com> <20170309175915.GA964@leoy-linaro> <3f27efee-3b63-81fd-eb96-73fd7e6f5e92@arm.com> <20170313165600.GB32431@linaro.org> <516f8989-4dde-2686-d549-0761feb14d1b@arm.com> <20170317101328.GA19877@leoy-linaro> <20170317155007.GA20435@linaro.org> <20170317162836.GD2020@leoy-linaro> <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> From: Mathieu Poirier Date: Mon, 20 Mar 2017 10:40:00 -0600 Message-ID: Subject: Re: [v3 3/5] coresight: add support for debug module To: Suzuki K Poulose Cc: Leo Yan , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Michael Turquette , Stephen Boyd , John Stultz , Guodong Xu , Haojian Zhuang , Greg Kroah-Hartman , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , linux-clk@vger.kernel.org, Mike Leach , Sudeep Holla Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17 March 2017 at 10:47, Suzuki K Poulose wrote: > On 17/03/17 16:28, Leo Yan wrote: >> >> On Fri, Mar 17, 2017 at 09:50:07AM -0600, Mathieu Poirier wrote: >> >> [...] >> >>> If we don't check for "nohlt" some platform may freeze, others may work. >>> If we >>> mandate that "nohlt" be present on the kernel cmd line it works in all >>> cases. >>> As such mandating that "nohlt" be present is a better way to go. >> >> >> Sure, so I will add below checking code in the probe function, please >> let me know if you have alter better way to implement this: >> >> + if (IS_ENABLED(CONFIG_CPU_IDLE) && >> + !strstr(boot_command_line, "nohlt")) { >> + dev_err(dev, "May not be accessible in CPU power >> domain.\n"); >> + return -EPERM; >> + } >> > > There is an API which kind of achieves what "nohlt" does at runtime : > > cpu_idle_poll_ctrl(true) > > So may be we could use that instead of depending on "nohlt". The other side > of the issues is "when do we decide to use the API". May be we could add > something > like : enable_debug, which could then trigger the panic notifier > registrations > and the above. That would still leave us with a case where the system > crashes > even before the user gets a terminal. May be the following is the best > option : > > 1) Dedicated kernel command line parameter for enabling the CPU debug at > boot/probe. > > and > > 2) Runtime enable method via sysfs. > > What do you think ? In my opinion booting with "nohlt" on the cmd line is sufficient to determine if we should use the driver or not. That way we also avoid declaring yet another sysfs flag, something I really want to avoid. Thanks, Mathieu > > Suzuki > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Poirier Subject: Re: [v3 3/5] coresight: add support for debug module Date: Mon, 20 Mar 2017 10:40:00 -0600 Message-ID: References: <1488520809-31670-4-git-send-email-leo.yan@linaro.org> <011fdac0-59bf-b539-2de3-0b59a41bc922@arm.com> <20170309175915.GA964@leoy-linaro> <3f27efee-3b63-81fd-eb96-73fd7e6f5e92@arm.com> <20170313165600.GB32431@linaro.org> <516f8989-4dde-2686-d549-0761feb14d1b@arm.com> <20170317101328.GA19877@leoy-linaro> <20170317155007.GA20435@linaro.org> <20170317162836.GD2020@leoy-linaro> <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Suzuki K Poulose Cc: Mark Rutland , "devicetree@vger.kernel.org" , Guodong Xu , Greg Kroah-Hartman , Catalin Marinas , Michael Turquette , Sudeep Holla , Will Deacon , "linux-kernel@vger.kernel.org" , Wei Xu , linux-clk@vger.kernel.org, Rob Herring , John Stultz , Haojian Zhuang , Leo Yan , Stephen Boyd , "linux-arm-kernel@lists.infradead.org" , Mike Leach List-Id: devicetree@vger.kernel.org On 17 March 2017 at 10:47, Suzuki K Poulose wrote: > On 17/03/17 16:28, Leo Yan wrote: >> >> On Fri, Mar 17, 2017 at 09:50:07AM -0600, Mathieu Poirier wrote: >> >> [...] >> >>> If we don't check for "nohlt" some platform may freeze, others may work. >>> If we >>> mandate that "nohlt" be present on the kernel cmd line it works in all >>> cases. >>> As such mandating that "nohlt" be present is a better way to go. >> >> >> Sure, so I will add below checking code in the probe function, please >> let me know if you have alter better way to implement this: >> >> + if (IS_ENABLED(CONFIG_CPU_IDLE) && >> + !strstr(boot_command_line, "nohlt")) { >> + dev_err(dev, "May not be accessible in CPU power >> domain.\n"); >> + return -EPERM; >> + } >> > > There is an API which kind of achieves what "nohlt" does at runtime : > > cpu_idle_poll_ctrl(true) > > So may be we could use that instead of depending on "nohlt". The other side > of the issues is "when do we decide to use the API". May be we could add > something > like : enable_debug, which could then trigger the panic notifier > registrations > and the above. That would still leave us with a case where the system > crashes > even before the user gets a terminal. May be the following is the best > option : > > 1) Dedicated kernel command line parameter for enabling the CPU debug at > boot/probe. > > and > > 2) Runtime enable method via sysfs. > > What do you think ? In my opinion booting with "nohlt" on the cmd line is sufficient to determine if we should use the driver or not. That way we also avoid declaring yet another sysfs flag, something I really want to avoid. Thanks, Mathieu > > Suzuki > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> References: <1488520809-31670-4-git-send-email-leo.yan@linaro.org> <011fdac0-59bf-b539-2de3-0b59a41bc922@arm.com> <20170309175915.GA964@leoy-linaro> <3f27efee-3b63-81fd-eb96-73fd7e6f5e92@arm.com> <20170313165600.GB32431@linaro.org> <516f8989-4dde-2686-d549-0761feb14d1b@arm.com> <20170317101328.GA19877@leoy-linaro> <20170317155007.GA20435@linaro.org> <20170317162836.GD2020@leoy-linaro> <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> From: Mathieu Poirier Date: Mon, 20 Mar 2017 10:40:00 -0600 Message-ID: Subject: Re: [v3 3/5] coresight: add support for debug module To: Suzuki K Poulose Cc: Leo Yan , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Michael Turquette , Stephen Boyd , John Stultz , Guodong Xu , Haojian Zhuang , Greg Kroah-Hartman , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , linux-clk@vger.kernel.org, Mike Leach , Sudeep Holla Content-Type: text/plain; charset=UTF-8 List-ID: On 17 March 2017 at 10:47, Suzuki K Poulose wrote: > On 17/03/17 16:28, Leo Yan wrote: >> >> On Fri, Mar 17, 2017 at 09:50:07AM -0600, Mathieu Poirier wrote: >> >> [...] >> >>> If we don't check for "nohlt" some platform may freeze, others may work. >>> If we >>> mandate that "nohlt" be present on the kernel cmd line it works in all >>> cases. >>> As such mandating that "nohlt" be present is a better way to go. >> >> >> Sure, so I will add below checking code in the probe function, please >> let me know if you have alter better way to implement this: >> >> + if (IS_ENABLED(CONFIG_CPU_IDLE) && >> + !strstr(boot_command_line, "nohlt")) { >> + dev_err(dev, "May not be accessible in CPU power >> domain.\n"); >> + return -EPERM; >> + } >> > > There is an API which kind of achieves what "nohlt" does at runtime : > > cpu_idle_poll_ctrl(true) > > So may be we could use that instead of depending on "nohlt". The other side > of the issues is "when do we decide to use the API". May be we could add > something > like : enable_debug, which could then trigger the panic notifier > registrations > and the above. That would still leave us with a case where the system > crashes > even before the user gets a terminal. May be the following is the best > option : > > 1) Dedicated kernel command line parameter for enabling the CPU debug at > boot/probe. > > and > > 2) Runtime enable method via sysfs. > > What do you think ? In my opinion booting with "nohlt" on the cmd line is sufficient to determine if we should use the driver or not. That way we also avoid declaring yet another sysfs flag, something I really want to avoid. Thanks, Mathieu > > Suzuki > From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.poirier@linaro.org (Mathieu Poirier) Date: Mon, 20 Mar 2017 10:40:00 -0600 Subject: [v3 3/5] coresight: add support for debug module In-Reply-To: <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> References: <1488520809-31670-4-git-send-email-leo.yan@linaro.org> <011fdac0-59bf-b539-2de3-0b59a41bc922@arm.com> <20170309175915.GA964@leoy-linaro> <3f27efee-3b63-81fd-eb96-73fd7e6f5e92@arm.com> <20170313165600.GB32431@linaro.org> <516f8989-4dde-2686-d549-0761feb14d1b@arm.com> <20170317101328.GA19877@leoy-linaro> <20170317155007.GA20435@linaro.org> <20170317162836.GD2020@leoy-linaro> <5995f6a7-08b1-c868-fc88-a145d1748a33@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 17 March 2017 at 10:47, Suzuki K Poulose wrote: > On 17/03/17 16:28, Leo Yan wrote: >> >> On Fri, Mar 17, 2017 at 09:50:07AM -0600, Mathieu Poirier wrote: >> >> [...] >> >>> If we don't check for "nohlt" some platform may freeze, others may work. >>> If we >>> mandate that "nohlt" be present on the kernel cmd line it works in all >>> cases. >>> As such mandating that "nohlt" be present is a better way to go. >> >> >> Sure, so I will add below checking code in the probe function, please >> let me know if you have alter better way to implement this: >> >> + if (IS_ENABLED(CONFIG_CPU_IDLE) && >> + !strstr(boot_command_line, "nohlt")) { >> + dev_err(dev, "May not be accessible in CPU power >> domain.\n"); >> + return -EPERM; >> + } >> > > There is an API which kind of achieves what "nohlt" does at runtime : > > cpu_idle_poll_ctrl(true) > > So may be we could use that instead of depending on "nohlt". The other side > of the issues is "when do we decide to use the API". May be we could add > something > like : enable_debug, which could then trigger the panic notifier > registrations > and the above. That would still leave us with a case where the system > crashes > even before the user gets a terminal. May be the following is the best > option : > > 1) Dedicated kernel command line parameter for enabling the CPU debug at > boot/probe. > > and > > 2) Runtime enable method via sysfs. > > What do you think ? In my opinion booting with "nohlt" on the cmd line is sufficient to determine if we should use the driver or not. That way we also avoid declaring yet another sysfs flag, something I really want to avoid. Thanks, Mathieu > > Suzuki >