From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751769AbbLIPcx (ORCPT ); Wed, 9 Dec 2015 10:32:53 -0500 Received: from mout.kundenserver.de ([212.227.126.187]:62942 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751598AbbLIPcv (ORCPT ); Wed, 9 Dec 2015 10:32:51 -0500 From: Arnd Bergmann To: xuejiancheng Cc: linux-arm-kernel@lists.infradead.org, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linux@arm.linux.org.uk, khilman@linaro.org, olof@lixom.net, xuwei5@hisilicon.com, haojian.zhuang@linaro.org, zhangfei.gao@linaro.org, bintian.wang@huawei.com, suwenping@hisilicon.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, yanhaifeng@hisilicon.com, gaofei@hisilicon.com, ml.yang@hisilicon.com, yanghongwei@hisilicon.com Subject: Re: [PATCH v2 4/9] ARM: dts: add dts files for hi3519-demb board Date: Wed, 09 Dec 2015 16:31:25 +0100 Message-ID: <5533669.m0KDLq8XOc@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <5666548B.90502@huawei.com> References: <1449110668-23647-1-git-send-email-xuejiancheng@huawei.com> <56652912.80308@huawei.com> <5666548B.90502@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:LAOjs1DTrLga2NomzKJh3oFMzX9NP9qLIYpFavvDQYimwJOagTX Q3VOQ//gAPgHKYYd0QCTE/cCf5ESMElrjxbSCRbbahxkjNHl/wQRh0w93Xq+8DVD/T7bQDw 0Y8ZTnxOtJtXUXxYQrJBpO7E0/msbnqEJRb7VBAz2brsV8vlMovmhFX30F92IKzX5gVuk7c 4bxv9KP873M1c6+SJs5Vg== X-UI-Out-Filterresults: notjunk:1;V01:K0:jnoHXsJy/74=:8Zobx/6O0wxrkZ/a88imVd QAQcmgQlSEYAxhm34qgNsknxRaBeR2N6bI23FgDdVDRXL8lhAuiXxvoyRx/FM/NCDxeEe8nuJ Vq47tWqHGD1pPuCFWy/FIZWxPWo91W156GiQviGXkzo4BX/7VgaOm45C8OUQ+L5RJzPOj3Yde vhoblxkU8uR6RTtoNeQH7HQMCVJIEaWOurvnIZySHIj9PW8Lcnr11yYe9BnI2/gPEtfeI0wW6 cA1Y83uMiG4WgjRH9fapUcwc+OhfNIncpMXqVV364gccjNB+7E+6k8Q7Om8vdNPS34qP3K/mf UDrumEqCAx3BaAgkSWTuVTeHqrseIBgUqHXykJdpEQnyYFmWnIXBsUrSERjyEcksoTx+YeKvT IFL3S7eeshf+mAoR4+S0Z5BQQesJ8FMdNazQnIpVz6RvFL3uKsasZV1medy/MRuZy2TrRcyaw AeLvVCXSjoMoWWxTpoMQOccxHHrLnb9w2W9uYOSuve8vt2Vs1M+pEh/5EvtnJatF6nmuTlJ/O DDImV6XDogOD7NFqF0FSTrF5ozk/BnIMQsfh/F8/rLrLDlYe5OmpEOQzwnfCtXoUxuuXIXOrz TZS8qDy5bzWSGrkE6uC+vseZs1phZEuPF/HhaAqToTRfr9xs4kxO0kjM7IZnl4kLTYXVbnyDM ri3+bNv1aTtWvk5lXYuVkVo4z4Tx+MzHLLB/oVJmqcM3WImBwkMTiNQnjnPChSYjQXobcMoF0 t0D9Xn7NfqOuyL7K Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 08 December 2015 11:54:51 xuejiancheng wrote: > On 2015/12/7 14:37, xuejiancheng wrote: > > > > On 2015/12/4 18:49, Arnd Bergmann wrote: > >> On Friday 04 December 2015 10:27:58 xuejiancheng wrote: > >>>> > >> Maybe split out the sysctrl binding from > >> Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt, as it has > >> you already have a couple of those, and it's not clear how they relate > >> to one another. > >> > >> If we introduce a string for all hip04 compatible sysctrl devices, we should > >> document that and use it consistently, so hi3519 becomes > >> > >> compatible = "hisilicon,hi3519-sysctrl", "hisilicon,hip04-sysctrl", "hisilicon,sysctrl"; > >> > >> but I'd clarify in the binding documentation that "hisilicon,sysctrl" should > >> only be used for hip04 and hi3519 but not the others. > >> > >> As this seems to be a standard part, we can also think about making a > >> high-level driver for in in drivers/soc rather than relying on the syscon > >> driver which we tend to use more for one-off devices with random register > >> layouts. > >> > > Sorry. I didn't understand your meaning well and maybe I gave you a wrong description. > > Please allow me to clarify it again. > > The "sysctrl" nodes here is just used for the "reboot" function. It is corresponding to > > the driver "drivers/power/reset/hisi-reboot.c". The compatible string in the driver is > > "hisilicon,sysctrl". > > The layout of this block is also different from the one in HiP04. > > I'll use "syscon" as the compatible value for sysctrl node and "syscon-reboot" for a new reboot node. > > This is not what I meant. You have to use "syscon" as the most generic "compatible" value here, but should add a machine specific string as a more specific one. "hisilicon,sysctrl" is not appropriate because it does not identify the IP block uniquely, you can only use that in combination with another more specific string. That way, we have to option to create a high-level driver for the IP block later if it turns out that we need some more generic functionality that is provided by those registers. Arnd