From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGGUX-0007Cm-Ac for qemu-devel@nongnu.org; Thu, 23 Jun 2016 22:05:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGGUV-0002g3-AV for qemu-devel@nongnu.org; Thu, 23 Jun 2016 22:05:08 -0400 Message-ID: <1466733871.23740.111.camel@aj.id.au> From: Andrew Jeffery Date: Fri, 24 Jun 2016 11:34:31 +0930 In-Reply-To: References: <1466648115-17015-1-git-send-email-andrew@aj.id.au> <1466648115-17015-2-git-send-email-andrew@aj.id.au> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-iCjhPVV1IdiIPjiWO/wB" Mime-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 1/3] hw/misc: Add a model for the ASPEED System Control Unit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: =?ISO-8859-1?Q?C=E9dric?= Le Goater , Joel Stanley , QEMU Developers , qemu-arm --=-iCjhPVV1IdiIPjiWO/wB Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2016-06-23 at 18:42 +0100, Peter Maydell wrote: > On 23 June 2016 at 03:15, Andrew Jeffery wrote: > >=20 > > The SCU is a collection of chip-level control registers that manage the > > various functions supported by ASPEED SoCs. Typically the bits control > > interactions with clocks, external hardware or reset behaviour, and we > > can largly take a hands-off approach to reads and writes. > >=20 > > Firmware makes heavy use of the state to determine how to boot, but the > > reset values vary from SoC to SoC (eg AST2400 vs AST2500). A qdev > > property is exposed so that the integrating SoC model can configure the > > silicon revision, which in-turn selects the appropriate reset values. > > Further qdev properties are exposed so the board model can configure th= e > > board-dependent hardware strapping. > >=20 > > Almost all provided AST2400 reset values are specified by the datasheet= . > > The notable exception is SOC_SCRATCH1, where we mark the DRAM as > > successfully initialised to avoid unnecessary dark corners in the SoC's > > u-boot support. > >=20 > > +static Property aspeed_scu_properties[] =3D { > > +=C2=A0=C2=A0=C2=A0=C2=A0DEFINE_PROP_UINT32("silicon-rev", AspeedSCUSta= te, silicon_rev, 0), > > +=C2=A0=C2=A0=C2=A0=C2=A0DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState= , hw_strap1, 0), > > +=C2=A0=C2=A0=C2=A0=C2=A0DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState= , hw_strap1, 0), > > +=C2=A0=C2=A0=C2=A0=C2=A0DEFINE_PROP_END_OF_LIST(), > > +}; > You don't seem to specify in the board layer or the SoC layer > any of these except hw-strap1, so should the default values > for these really all be zero? Both strap register default values are 0 according to the datasheet. >=20 > I suspect silicon-rev at least should either have a default > value specified here, or have the SoC layer specify it. > (It probably should not be specified at the board level.) I intended to set silicon-rev in the SoC layer, so I'll fix patch 2/3. With the addition of sanity checking in the SCU's realise() we'll catch the case where it's an invalid value (eg 0). I don't think it's right to plow ahead with an unexpected configuration if a chosen default value doesn't match the SoC at hand. Maybe I shouldn't send patches with a heavy head cold :/ Cheers, Andrew --=-iCjhPVV1IdiIPjiWO/wB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCgAGBQJXbJUvAAoJEJ0dnzgO5LT5I4oP/20HU2XwQO2RjAvucWTOPteq cIHuRwxZY7rkcApdOgGs/1IXHfJ/PIr5wO93ql+Bz3jXLIYb/g8KNQc/KIfB9RjZ ywANoO93tH9XscHLWJ3h+qvJ54t8q8VusXrPm0wN3aZB9PC63eUfWMKmJyTP/Pzd Ps39VUzRTfc1o/NAcHo8M3sTobo/lyk4Z25FirFhnVvDz+g0drialgXMo8NTEHrD nrA85KtxlOl74CoCQTk/GY3+NiqA3JSGVJ9rRxwIgl8sTQxZfjHcMaJnnXCLk2DO 6OCfjtDpljbVI6Hdq2fswvEeoEuif6Mz/Ki0cSAO060kb+n+R+S+rkwpfv+3zllF 8ILWFHV+JUNjrAB7nL8Hjkq2u1Pub1uQVNI14SJp9R4UU9Im2qKly1l0tn4fCXat YLUxho0Xdq0hCPks3uLIrR6rqpMMM7gpytIXa7q0KDbtVt10H2AOKJ0tn6fZteDP wo0BkzhR6aUxtKSIdgNZlDGctrBGGpsYKyvETl6Z7ds7+ZTev0lkdv7DbS6zeORS 5/sduvLP13FWNqA6Kyz2J+iXITowPJaEbLtW7uQyxgA3qZkcU3zj/t9UudhGDVqS pnqF8UEl3xTS7hBeoXMeZnxYQf7CLRa8T2x002n5YOFT0O5cELg9m8Mkg5NzJsIn 39zUcQMu6SX/fwcjgCQo =encd -----END PGP SIGNATURE----- --=-iCjhPVV1IdiIPjiWO/wB--