All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: "Rafał Miłecki" <zajec5@gmail.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>
Cc: "Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com,
	"Rafał Miłecki" <rafal@milecki.pl>
Subject: Re: [PATCH V2 2/2] arm64: dts: broadcom: add BCM4908 and Asus GT-AC5300 early DTS files
Date: Wed, 11 Nov 2020 07:33:58 -0800	[thread overview]
Message-ID: <1d21b0ee-f880-746f-5dc4-5290aec1eb7e@gmail.com> (raw)
In-Reply-To: <1d62eeca-dc09-866c-85c7-235144f8e782@gmail.com>



On 11/10/2020 9:59 PM, Rafał Miłecki wrote:
> On 11.11.2020 02:04, Florian Fainelli wrote:
>>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> new file mode 100644
>>> index 000000000000..3bbefc86b978
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> @@ -0,0 +1,188 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>> +
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +    interrupt-parent = <&gic>;
>>> +
>>> +    #address-cells = <2>;
>>> +    #size-cells = <2>;
>>> +
>>> +    aliases {
>>> +        serial0 = &uart0;
>>> +    };
>>> +
>>> +    chosen {
>>> +        bootargs = "earlycon=bcm63xx_uart,0xff800640";
>>
>> We talked about it before, but the earlycon should be dropped from the
>> .dtsi file, it does not really belong there.
> 
> I asked the following question that you probably missed, could you check
> it, please?
> 
> On Wed, 4 Nov 2020 at 09:02, Rafał Miłecki <zajec5@gmail.com> wrote:
>> Can you explain why, is that some kernel rule I missed? That's
> extremely helpful for debugging.

It's useful for debugging but because it is meant for debugging it does
not really belong in a .dtsi which gets included by a board level .dts
file and I would argue that it does not belong in a board level .dts
either. This is something that you can and should keep locally while
debugging and remove for "production". That is not a rule that is
written somewhere, and there are certainly cases of .dts files in the
kernel containing "earlycon" for better or for worse.
-- 
Florian

WARNING: multiple messages have this Message-ID (diff)
From: Florian Fainelli <f.fainelli@gmail.com>
To: "Rafał Miłecki" <zajec5@gmail.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>
Cc: devicetree@vger.kernel.org,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	"Rafał Miłecki" <rafal@milecki.pl>,
	"Will Deacon" <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V2 2/2] arm64: dts: broadcom: add BCM4908 and Asus GT-AC5300 early DTS files
Date: Wed, 11 Nov 2020 07:33:58 -0800	[thread overview]
Message-ID: <1d21b0ee-f880-746f-5dc4-5290aec1eb7e@gmail.com> (raw)
In-Reply-To: <1d62eeca-dc09-866c-85c7-235144f8e782@gmail.com>



On 11/10/2020 9:59 PM, Rafał Miłecki wrote:
> On 11.11.2020 02:04, Florian Fainelli wrote:
>>> diff --git a/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> new file mode 100644
>>> index 000000000000..3bbefc86b978
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/broadcom/bcm4908/bcm4908.dtsi
>>> @@ -0,0 +1,188 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>> +
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +    interrupt-parent = <&gic>;
>>> +
>>> +    #address-cells = <2>;
>>> +    #size-cells = <2>;
>>> +
>>> +    aliases {
>>> +        serial0 = &uart0;
>>> +    };
>>> +
>>> +    chosen {
>>> +        bootargs = "earlycon=bcm63xx_uart,0xff800640";
>>
>> We talked about it before, but the earlycon should be dropped from the
>> .dtsi file, it does not really belong there.
> 
> I asked the following question that you probably missed, could you check
> it, please?
> 
> On Wed, 4 Nov 2020 at 09:02, Rafał Miłecki <zajec5@gmail.com> wrote:
>> Can you explain why, is that some kernel rule I missed? That's
> extremely helpful for debugging.

It's useful for debugging but because it is meant for debugging it does
not really belong in a .dtsi which gets included by a board level .dts
file and I would argue that it does not belong in a board level .dts
either. This is something that you can and should keep locally while
debugging and remove for "production". That is not a rule that is
written somewhere, and there are certainly cases of .dts files in the
kernel containing "earlycon" for better or for worse.
-- 
Florian

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

  reply	other threads:[~2020-11-11 15:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05  8:54 [PATCH V2 1/2] arm64: add config for Broadcom BCM4908 SoCs Rafał Miłecki
2020-11-05  8:54 ` Rafał Miłecki
2020-11-05  8:54 ` [PATCH V2 2/2] arm64: dts: broadcom: add BCM4908 and Asus GT-AC5300 early DTS files Rafał Miłecki
2020-11-05  8:54   ` Rafał Miłecki
2020-11-11  1:04   ` Florian Fainelli
2020-11-11  1:04     ` Florian Fainelli
2020-11-11  5:59     ` Rafał Miłecki
2020-11-11  5:59       ` Rafał Miłecki
2020-11-11 15:33       ` Florian Fainelli [this message]
2020-11-11 15:33         ` Florian Fainelli

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=1d21b0ee-f880-746f-5dc4-5290aec1eb7e@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=catalin.marinas@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=rafal@milecki.pl \
    --cc=robh+dt@kernel.org \
    --cc=will@kernel.org \
    --cc=zajec5@gmail.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.