Subject: Re: [PATCH v1] ARM: dts: sun7i: provide ramoops region on BananaPi
To: Maxime Ripard <maxime.ripard@free-electrons.com>
References: <1498065017-30556-1-git-send-email-linux@rempel-privat.de>
 <20170622061617.5guhcyxdthvmiq7j@flea.lan>
Cc: Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
        Russell King <linux@armlinux.org.uk>, Chen-Yu Tsai <wens@csie.org>,
        Kees Cook <keescook@chromium.org>, Anton Vorontsov <anton@enomsg.org>,
        Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
From: Oleksij Rempel <linux@rempel-privat.de>
Message-ID: <48c81184-3713-31a8-03e8-f55ffaef639c@rempel-privat.de>
Date: Thu, 22 Jun 2017 08:40:44 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <20170622061617.5guhcyxdthvmiq7j@flea.lan>
Content-Type: multipart/signed; micalg=pgp-sha256;
 protocol="application/pgp-signature";
 boundary="Vk9fo16Qcoxfjrp9mLSuJre2Aem7GfaXl"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4195
Lines: 116

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--Vk9fo16Qcoxfjrp9mLSuJre2Aem7GfaXl
Content-Type: multipart/mixed; boundary="RUM1U6ePKkIcEvxuOjrHGuuk3CKINcLFk";
 protected-headers="v1"
From: Oleksij Rempel <linux@rempel-privat.de>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
 Russell King <linux@armlinux.org.uk>, Chen-Yu Tsai <wens@csie.org>,
 Kees Cook <keescook@chromium.org>, Anton Vorontsov <anton@enomsg.org>,
 Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>,
 devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-kernel@vger.kernel.org
Message-ID: <48c81184-3713-31a8-03e8-f55ffaef639c@rempel-privat.de>
Subject: Re: [PATCH v1] ARM: dts: sun7i: provide ramoops region on BananaPi
References: <1498065017-30556-1-git-send-email-linux@rempel-privat.de>
 <20170622061617.5guhcyxdthvmiq7j@flea.lan>
In-Reply-To: <20170622061617.5guhcyxdthvmiq7j@flea.lan>

--RUM1U6ePKkIcEvxuOjrHGuuk3CKINcLFk
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

Am 22.06.2017 um 08:16 schrieb Maxime Ripard:
> Hi Oleksij,
>=20
> On Wed, Jun 21, 2017 at 07:10:17PM +0200, Oleksij Rempel wrote:
>> This should help provide useful debug information on remote
>> targets without UART.
>>
>> Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
>> ---
>>  arch/arm/boot/dts/sun7i-a20-bananapi.dts | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/=
dts/sun7i-a20-bananapi.dts
>> index ed2f35a..38923bf 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> @@ -63,6 +63,22 @@
>>  		stdout-path =3D "serial0:115200n8";
>>  	};
>> =20
>> +	reserved-memory {
>> +		#address-cells =3D <1>;
>> +		#size-cells =3D <1>;
>> +		ranges;
>> +
>> +		ramoops@7fb6a000 {
>> +			compatible =3D "ramoops";
>> +			reg =3D <0x7fb6a000 0x100000>;
>> +			ecc-size	=3D <16>;
>> +			record-size	=3D <0x00020000>;
>> +			console-size	=3D <0x00020000>;
>> +			ftrace-size	=3D <0x00020000>;
>> +			pmsg-size	=3D <0x00020000>;
>> +		};
>> +	};
>> +
>=20
> I'm a bit skeptical about this one.

I can understand your concern.

> First, there's nothing specific to the bananapi,

It is specific to memory size, bootloader used for the board and in some
cases security co processor. Any thing else I forgot?

> but every one will> want to have different sizes here, or different par=
ameters.

I don't thing every one need different configurations. I assume current
parameters are good enough for most users. But if users need some thing
different they should be overwritten by kernel boot args.

May be, DTS should provide some generic/ramoops area and let
distributions decide how to use it? For example compile kernel with some
defaults for ramoops.

> Since using the kernel parameters is also an option, I'd rather
> document how to do that with the parameters.

Here are my arguments to do this in DTS:
- some issues are hardly reproducible and it is good to catch it as soon
as possible. So it should be enabled before first problem will happen.
- my previous experience with pstore was a bit disappointing, it didn't
worked on SMP system correctly. So the test coverage seems to be
minimal. Probably because it is not enabled by default.
- at least barebox can extract some postmortem information from not
booting system. Bootloader and Kernel need to use same parameters,
devicetree seems to be perfect here.
- distributions should be able to use this per default as soon as this
option is available.

--=20
Regards,
Oleksij


--RUM1U6ePKkIcEvxuOjrHGuuk3CKINcLFk--

--Vk9fo16Qcoxfjrp9mLSuJre2Aem7GfaXl
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF4EAREIAAYFAllLZnIACgkQHwImuRkmbWmTVQEAmfnVRopApw8bN8iklxtbhuWI
xxH6xYaqWMPix0WeHbMA/0r9eV+imU+bxDxo5bQV8EJvCBA9iKpdf711wgOqMU5J
=EQHj
-----END PGP SIGNATURE-----

--Vk9fo16Qcoxfjrp9mLSuJre2Aem7GfaXl--