From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suzuki K Poulose Subject: Re: [PATCH v6 6/8] coresight: add support for CPU debug module Date: Wed, 19 Apr 2017 15:32:39 +0100 Message-ID: References: <1491485461-22800-1-git-send-email-leo.yan@linaro.org> <1491485461-22800-7-git-send-email-leo.yan@linaro.org> <5c5cb6f8-1dcb-8a9d-1605-c006656005eb@arm.com> <20170419142812.GA16160@leoy-linaro> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170419142812.GA16160@leoy-linaro> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Leo Yan Cc: Jonathan Corbet , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Andy Gross , David Brown , Mathieu Poirier , Stephen Boyd , linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mike Leach , Sudeep Holla List-Id: linux-arm-msm@vger.kernel.org On 19/04/17 15:28, Leo Yan wrote: > Hi Suzuki, > > On Wed, Apr 19, 2017 at 02:23:04PM +0100, Suzuki K Poulose wrote: >> Hi Leo, >> >> This version looks good to me. I have two minor comments below. > > Thanks for reviewing. Will take the suggestions. Just check a bit for > last comment. > > [...] > >>> +static int debug_probe(struct amba_device *adev, const struct amba_id *id) >>> +{ >>> + void __iomem *base; >>> + struct device *dev = &adev->dev; >>> + struct debug_drvdata *drvdata; >>> + struct resource *res = &adev->res; >>> + struct device_node *np = adev->dev.of_node; >>> + int ret; >>> + >>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >>> + if (!drvdata) >>> + return -ENOMEM; >>> + >>> + drvdata->cpu = np ? of_coresight_get_cpu(np) : 0; >>> + if (per_cpu(debug_drvdata, drvdata->cpu)) { >>> + dev_err(dev, "CPU%d drvdata has been initialized\n", >>> + drvdata->cpu); >> >> May be we could warn about a possible issue in the DT ? > > So can I understand the suggestion is to add warning in function > of_coresight_get_cpu() when cannot find CPU number, and here directly > bail out? No. One could have single CPU DT, where he doesn't need to provide the CPU number. Hence, it doesn't make sense to WARN in of_coresight_get_cpu(). But when we hit the case above, we find that the some node was registered for the given CPU (be it 0 or any other), which is definitely an error in DT. Due to 1) Hasn't specified the CPU number for more than one node OR 2) CPU number duplicated in the more than one nodes. Cheers Suzuki -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764740AbdDSOct (ORCPT ); Wed, 19 Apr 2017 10:32:49 -0400 Received: from foss.arm.com ([217.140.101.70]:41096 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1764033AbdDSOco (ORCPT ); Wed, 19 Apr 2017 10:32:44 -0400 Subject: Re: [PATCH v6 6/8] coresight: add support for CPU debug module To: Leo Yan References: <1491485461-22800-1-git-send-email-leo.yan@linaro.org> <1491485461-22800-7-git-send-email-leo.yan@linaro.org> <5c5cb6f8-1dcb-8a9d-1605-c006656005eb@arm.com> <20170419142812.GA16160@leoy-linaro> Cc: Jonathan Corbet , Rob Herring , Mark Rutland , Wei Xu , Catalin Marinas , Will Deacon , Andy Gross , David Brown , Mathieu Poirier , Stephen Boyd , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, Mike Leach , Sudeep Holla From: Suzuki K Poulose Message-ID: Date: Wed, 19 Apr 2017 15:32:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170419142812.GA16160@leoy-linaro> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/04/17 15:28, Leo Yan wrote: > Hi Suzuki, > > On Wed, Apr 19, 2017 at 02:23:04PM +0100, Suzuki K Poulose wrote: >> Hi Leo, >> >> This version looks good to me. I have two minor comments below. > > Thanks for reviewing. Will take the suggestions. Just check a bit for > last comment. > > [...] > >>> +static int debug_probe(struct amba_device *adev, const struct amba_id *id) >>> +{ >>> + void __iomem *base; >>> + struct device *dev = &adev->dev; >>> + struct debug_drvdata *drvdata; >>> + struct resource *res = &adev->res; >>> + struct device_node *np = adev->dev.of_node; >>> + int ret; >>> + >>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >>> + if (!drvdata) >>> + return -ENOMEM; >>> + >>> + drvdata->cpu = np ? of_coresight_get_cpu(np) : 0; >>> + if (per_cpu(debug_drvdata, drvdata->cpu)) { >>> + dev_err(dev, "CPU%d drvdata has been initialized\n", >>> + drvdata->cpu); >> >> May be we could warn about a possible issue in the DT ? > > So can I understand the suggestion is to add warning in function > of_coresight_get_cpu() when cannot find CPU number, and here directly > bail out? No. One could have single CPU DT, where he doesn't need to provide the CPU number. Hence, it doesn't make sense to WARN in of_coresight_get_cpu(). But when we hit the case above, we find that the some node was registered for the given CPU (be it 0 or any other), which is definitely an error in DT. Due to 1) Hasn't specified the CPU number for more than one node OR 2) CPU number duplicated in the more than one nodes. Cheers Suzuki From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suzuki.Poulose@arm.com (Suzuki K Poulose) Date: Wed, 19 Apr 2017 15:32:39 +0100 Subject: [PATCH v6 6/8] coresight: add support for CPU debug module In-Reply-To: <20170419142812.GA16160@leoy-linaro> References: <1491485461-22800-1-git-send-email-leo.yan@linaro.org> <1491485461-22800-7-git-send-email-leo.yan@linaro.org> <5c5cb6f8-1dcb-8a9d-1605-c006656005eb@arm.com> <20170419142812.GA16160@leoy-linaro> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 19/04/17 15:28, Leo Yan wrote: > Hi Suzuki, > > On Wed, Apr 19, 2017 at 02:23:04PM +0100, Suzuki K Poulose wrote: >> Hi Leo, >> >> This version looks good to me. I have two minor comments below. > > Thanks for reviewing. Will take the suggestions. Just check a bit for > last comment. > > [...] > >>> +static int debug_probe(struct amba_device *adev, const struct amba_id *id) >>> +{ >>> + void __iomem *base; >>> + struct device *dev = &adev->dev; >>> + struct debug_drvdata *drvdata; >>> + struct resource *res = &adev->res; >>> + struct device_node *np = adev->dev.of_node; >>> + int ret; >>> + >>> + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); >>> + if (!drvdata) >>> + return -ENOMEM; >>> + >>> + drvdata->cpu = np ? of_coresight_get_cpu(np) : 0; >>> + if (per_cpu(debug_drvdata, drvdata->cpu)) { >>> + dev_err(dev, "CPU%d drvdata has been initialized\n", >>> + drvdata->cpu); >> >> May be we could warn about a possible issue in the DT ? > > So can I understand the suggestion is to add warning in function > of_coresight_get_cpu() when cannot find CPU number, and here directly > bail out? No. One could have single CPU DT, where he doesn't need to provide the CPU number. Hence, it doesn't make sense to WARN in of_coresight_get_cpu(). But when we hit the case above, we find that the some node was registered for the given CPU (be it 0 or any other), which is definitely an error in DT. Due to 1) Hasn't specified the CPU number for more than one node OR 2) CPU number duplicated in the more than one nodes. Cheers Suzuki