From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753255AbcGLJGX (ORCPT ); Tue, 12 Jul 2016 05:06:23 -0400 Received: from m199-177.yeah.net ([123.58.177.199]:58110 "EHLO m199-177.yeah.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751886AbcGLJGS (ORCPT ); Tue, 12 Jul 2016 05:06:18 -0400 Subject: Re: [PATCH v2 06/10] soc: Add SoC specific driver support for nuc900 To: Arnd Bergmann , linux-arm-kernel@lists.infradead.org References: <1468135649-19980-1-git-send-email-vw@iommu.org> <1468135649-19980-7-git-send-email-vw@iommu.org> <4765191.NGpMC1JR46@wuerfel> Cc: devicetree@vger.kernel.org, jason@lakedaemon.net, Wan Zongshun , Daniel Lezcano , Russell King , linux-kernel@vger.kernel.org, p.zabel@pengutronix.de, Thomas Gleixner , linux-clk@vger.kernel.org From: Wan Zongshun Message-ID: <5784B302.80307@iommu.org> Date: Tue, 12 Jul 2016 17:06:10 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <4765191.NGpMC1JR46@wuerfel> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Spam-Status: e1koWUFPN1dZCBgUCR5ZQVZIVUhISUJCQkJPT01DTUNDQk1XWQkOFx4IWU FZKCs9JDU0NT4kMjUkMz46Pz4pQUtVS0A2IyQiPigkMjUkMz46Pz4pQUtVS0ArLykkNTQkMjUkMz 46Pz4pQUtVS0A4NC41LykiJDg1QUtVS0ApPjwyNDUkOigyOkFLVUtAKyk0LTI1OD4kKDM6NTwzOj JBS1VLQD8iNTo2MjgkMiskNTQkMjUkMz46Pz4pQUtVS0A2LjcvMiQpOCsvJD8yPT0#KT41LyQyNS QzPjo*PilBSVVLQDIrJC80PzoiJDg1LyRLJEpLS0FLVUtAMiskSiQ2MjUuLz4kODUvJEskSktBS1 VLQDIrJEhLJDYyNS4vPiQ4NS8kSyROS0FLVUtAMiskTiQ2MjUuLz4kODUvJEskSktBS1VLQDIrJE okMzQuKSQ4NS8kSyRKS0tBS1VLQCguOTE#OC8kLzQ*OiIkODUvJEskSktLQUtVS0AoLjkxPjgvJE okMzQuKSQ4NS8kSyRKS0tBS1VLQCguOTE#OC8kTiQ2MjUuLz4kODUvJEskSktBS1VLQCg5MSQ3Pj UkT0skI0FLVUtLSktALz4jLyQ3PjUkSklDSyQjQUtVS0tKS0A4NS8kMy82NyQ3PjUkSjAkTjBBS1 VLS0pLQCk#OD4yLT4pJDg1LyROJCNBS1VLS0pLQCM2OjI3PikkNS43N0FLVUtLSktAPz43Mi0#KS Q3PjUkS0FLVUtLSktANzo1PCQ#NUFLVUtLSktAPTUkNjoiJE9KQiQzNzEkSiRLQ0tIS09BS1VISE A9KyQpPiQ9LCQzNzEkS0NLSEtNQVZMVU5APTUkKC45JD41LDQpPygkMzcxJEpLS0lLSkFLVUlDQC guOSQ#QUpVTk5ZBg++ X-HM-Sender-Digest: e1kSHx4VD1lBWUc6KxA6Fgw4PDoYLjIBPE0VGCozChYaCwxVSlVKT01D SEpPSExKTkhMVTMWGhIXVQ0MOxIUFhYOVRQJHEVZV1kMHhlZQR0aFwgeV1kIAVlBT0tNTjdXWRIL WUFZSUpLVUpIVUJMVUpNQ1kG X-HM-Tid: 0a55de5b454e6427e1d73110573f Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年07月11日 16:03, Arnd Bergmann wrote: > On Sunday, July 10, 2016 3:27:26 PM CEST Wan Zongshun wrote: >> + ret = of_property_read_string(np, "compatible", &soc_dev_attr->soc_id); >> + if (ret) >> return -EINVAL; >> + >> + soc_dev_attr->machine = "NUC900EVB"; >> + soc_dev_attr->family = "NUC900"; >> + soc_dev = soc_device_register(soc_dev_attr); >> + if (IS_ERR(soc_dev)) { >> + kfree(soc_dev_attr); >> + return -ENODEV; >> + } >> + >> + ret = regmap_read(syscon_regmap, GCR_CHIPID, &nuc900_chipid); >> + if (ret) >> + return -ENODEV; >> + >> + device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr); >> + device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr); >> + >> + dev_info(&pdev->dev, "Nuvoton Chip ID: 0x%x, Version ID:0x%x\n", >> + nuc900_chipid & GCR_CHIPID_MASK, >> + (nuc900_chipid >> 24) & 0xff); > > I'm still a bit unsure about the set of attributes here. > > - The "soc_id" is read from the device tree from the field that contains > the board name, I think for consistency you should try to map the > GCR_CHIPID to the name of the SoC and assign that here > > - The "machine" is hardcoded to "NUC900EVB", which in turn looks like > a particular board but not the one you are running on. Maybe read > that from the DT instead? > > - The "revision" is not filled at all, I would suggest using something > derived from the GCR_CHIPID register here > > - you have two nonstandard attributes "chipid" and "version", which > I'd hope to avoid -- the set of standard attributes is supposed to > give enough information about the machine, and platform independent > user space will never read those. So, Maybe I can remove those two codes, no need push those information to user space? device_create_file(soc_device_to_device(soc_dev), &nuc900_chipid_attr); device_create_file(soc_device_to_device(soc_dev), &nuc900_version_attr); > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >