From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754365AbcKIQ41 (ORCPT ); Wed, 9 Nov 2016 11:56:27 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:64160 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753311AbcKIQ4Z (ORCPT ); Wed, 9 Nov 2016 11:56:25 -0500 From: Arnd Bergmann To: linuxppc-dev@lists.ozlabs.org Cc: Geert Uytterhoeven , Greg Kroah-Hartman , Yangbo Lu , Simon Horman , Magnus Damm , Rob Herring , Mark Rutland , devicetree@vger.kernel.org, Dirk Behme , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus Date: Wed, 09 Nov 2016 17:55:28 +0100 Message-ID: <3693445.TSdZGaiT9S@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <1477913455-5314-8-git-send-email-geert+renesas@glider.be> References: <1477913455-5314-1-git-send-email-geert+renesas@glider.be> <1477913455-5314-8-git-send-email-geert+renesas@glider.be> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:CKESYbi2g7+eh4BesVmJpxSGubpW4xrTVgh4TdbYs85pqjLetxx Fv5GQdF+SiNwQYkw40T6ENygQeoUMF5d3VJ4pxMWdV0cqgwjgskkZ9vupWXulP6WrhnQY4H V3tO26PgOFRD5TPHYmPWv5oyEAOC4sK1dyvJ1Sdz5Q1HgCRRIfzi/5sQEs40IyMmmfYMPjy ctkNTpByFc71o+0idsVqw== X-UI-Out-Filterresults: notjunk:1;V01:K0:ke1lQ68Ccrk=:TxSLea60p0hKuo7MftfvPX WI1EYfID5ZHZrU/u+KUcj5PYHGonKH8jGrk8s2gRiTrCkUVYvkJe686gVUpMyqDyINWIhkO3l sInK5pDu2RC3xyCeI5MEAnjRqVzYgmqQvjoD7h/R6TEmVwGAXzH0dqzwjuU5hK9Rum7ECPNUv J0ESzlh4jw9WgD+vbsngFwSvVKLXhHeZLp4f7muZP/Ti/SzV2Ol3sqgaiNX0XOL0kQClLUXsJ iG+sL0rGABp+D5PqY23kuL2/YSKo++xSjSsxDY+Iw/2BKkUbCdmE+y7dpjAv7rW2VZP1Oebuq rzFRQrU+WlRIpjYvN5l8l22yT6O6cVuUPJ/L1Ft7eKMHwIikhCD0cN9uOTweW1v+2GzoLdTgP sVz7uZ7DjAhqsAcOZmZeaCUaThcvb194Bcu0iBW4S7MjsgJZc6sAoVlvoXtzfkyNJMb082Gi2 VJDT1PRbrtcoEfpveVe9ksfjWnn2Lk2LLXTm35exS4WYEXqTzNx9EEedFByrHBU95Z4mI4C9R Kmg2uW4bT/t4hyD9UMOKoNcpdgp49P6S/qB0NRe0dmfVr+psK1w1XI/bEmqjYqBMzD4S7kKdM PcmrhkqfcsbOzVqTIPTROrRxHQWA5/EQzeSefS1YivspNjF4qrChdfGA7+/t8Vp5C1jIKE4k/ f0kWYiN/u6WcAHW3HeGup3ew0OfGrCk2hIx217YDvyf040fHfPjTs37VGYagSrijVyoC0cvSh 3Oxm2TN7Fw+rVqCE Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote: > v2: > - Drop SoC families and family names; use fixed "Renesas" instead, I think I'd rather have seen the family names left in there, but it's not important, so up to you. > - Use "renesas,prr" and "renesas,cccr" device nodes in DT if > available, else fall back to hardcoded addresses for compatibility > with existing DTBs, I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT binding for these, among other things. It does seem wrong to have a device node for a specific register though. Shouldn't the node be for the block of registers that these are inside of? > - Don't register the SoC bus if the chip ID register is missing, Why? My objection was to hardcoding the register, not to registering the device? I think I'd rather see the device registered with an empty revision string. > +#define CCCR 0xe600101c /* Common Chip Code Register */ > +#define PRR 0xff000044 /* Product Register */ > +#define PRR3 0xfff00044 /* Product Register on R-Car Gen3 */ > + > +static const struct of_device_id renesas_socs[] __initconst = { > +#ifdef CONFIG_ARCH_R8A73A4 > + { .compatible = "renesas,r8a73a4", .data = (void *)PRR, }, > +#endif > +#ifdef CONFIG_ARCH_R8A7740 > + { .compatible = "renesas,r8a7740", .data = (void *)CCCR, }, > +#endif My preference here would be to list the register address only for SoCs that are known to need them, while also having .dtb files that don't have the nodes. > +static int __init renesas_soc_init(void) > +{ > + struct soc_device_attribute *soc_dev_attr; > + const struct of_device_id *match; > + void __iomem *chipid = NULL; > + struct soc_device *soc_dev; > + struct device_node *np; > + unsigned int product; > + > + np = of_find_matching_node_and_match(NULL, renesas_socs, &match); > + if (!np) > + return -ENODEV; > + > + of_node_put(np); > + > + /* Try PRR first, then CCCR, then hardcoded fallback */ > + np = of_find_compatible_node(NULL, NULL, "renesas,prr"); > + if (!np) > + np = of_find_compatible_node(NULL, NULL, "renesas,cccr"); > + if (np) { > + chipid = of_iomap(np, 0); > + of_node_put(np); > + } else if (match->data) { > + chipid = ioremap((uintptr_t)match->data, 4); > + } > + if (!chipid) > Here, I'd turn the order around and look for the DT nodes of the devices first. Only if they are not found, look at the compatible string of the root node. No need to search for a node though, you know which one it is when you look for a compatible = "renesas,r8a73a4". Arnd