Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756467AbcKWWsR (ORCPT ); Wed, 23 Nov 2016 17:48:17 -0500 Received: from vern.gendns.com ([206.190.152.46]:37444 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933626AbcKWWsP (ORCPT ); Wed, 23 Nov 2016 17:48:15 -0500 Subject: Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes To: Kevin Hilman References: <1479207611-18028-1-git-send-email-bgolaszewski@baylibre.com> <5e647eb0-2f8a-b46b-2048-7616bfb54ad7@lechnology.com> Cc: Bartosz Golaszewski , Michael Turquette , Sekhar Nori , Rob Herring , Frank Rowand , Mark Rutland , Peter Ujfalusi , Russell King , linux-devicetree , David Airlie , LKML , linux-drm , Tomi Valkeinen , Jyri Sarha , arm-soc , Laurent Pinchart From: David Lechner Message-ID: Date: Wed, 23 Nov 2016 16:48:11 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3100 Lines: 95 On 11/23/2016 04:32 PM, Kevin Hilman wrote: > David Lechner writes: > >> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >>> 2016-11-22 23:23 GMT+01:00 David Lechner : >>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>>> >>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>>> controller drivers to da850.dtsi. >>>>> >>>>> Signed-off-by: Bartosz Golaszewski >>>>> --- >>>>> v1 -> v2: >>>>> - moved the priority controller node above the cfgchip node >>>>> - renamed added nodes to better reflect their purpose >>>>> >>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >>>>> index 1bb1f6d..412eec6 100644 >>>>> --- a/arch/arm/boot/dts/da850.dtsi >>>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>>> @@ -210,6 +210,10 @@ >>>>> }; >>>>> >>>>> }; >>>>> + prictrl: priority-controller@14110 { >>>>> + compatible = "ti,da850-mstpri"; >>>>> + reg = <0x14110 0x0c>; >>>> >>>> >>>> I think we should add status = "disabled"; here and let boards opt in. >>>> >>>>> + }; >>>>> cfgchip: chip-controller@1417c { >>>>> compatible = "ti,da830-cfgchip", "syscon", >>>>> "simple-mfd"; >>>>> reg = <0x1417c 0x14>; >>>>> @@ -451,4 +455,8 @@ >>>>> 1 0 0x68000000 0x00008000>; >>>>> status = "disabled"; >>>>> }; >>>>> + memctrl: memory-controller@b0000000 { >>>>> + compatible = "ti,da850-ddr-controller"; >>>>> + reg = <0xb0000000 0xe8>; >>>> >>>> >>>> same here. status = "disabled"; >>>> >>>>> + }; >>>>> }; >>>>> >>> >>> Hi David, >>> >>> I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4]. >>> >>> FYI this patch has already been queued by Sekhar. >> >> Thanks. I did not see those threads. >> >> FYI to maintainers, having these enabled by default causes error >> messages in the kernel log for other boards that are not supported by >> the drivers. > > Then the driver is too noisy and should be cleaned up. > >> Since there is only one board that is supported and soon >> to be 2 that are not, I would rather have this disabled by default to >> avoid the error messages. > > IMO, what exactly are the error messages? Sounds like the driver is > being too verbose, and calling things errors that are not really errors. It is just one line per driver. dev_err(dev, "no master priorities defined for this board\n"); and dev_err(dev, "no settings defined for this board\n"); Since "ti,da850-lcdk" is the only board supported in these drivers, all other boards will see these error messages. Also, these modules will be loaded and taking up memory on boards that don't use them. This not really a big deal and they can be explicitly disabled, so maybe it was not worth mentioning. > > Kevin >