From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhangfei.gao@linaro.org (zhangfei) Date: Fri, 20 Jun 2014 13:56:54 +0800 Subject: [PATCH RFC 1/2] libahci_platform: add ahci_platform_get_of_property In-Reply-To: <53A293CB.3040704@redhat.com> References: <1403067249-8021-1-git-send-email-zhangfei.gao@linaro.org> <1403067249-8021-2-git-send-email-zhangfei.gao@linaro.org> <20140618140352.GD11895@leverpostej> <53A24950.2020300@linaro.org> <53A293CB.3040704@redhat.com> Message-ID: <53A3CD26.3070001@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 06/19/2014 03:39 PM, Hans de Goede wrote: > Hi, > > On 06/19/2014 04:22 AM, zhangfei wrote: >> Hi Mark, >> >> On 06/18/2014 10:03 PM, Mark Rutland wrote: >>> On Wed, Jun 18, 2014 at 05:54:08AM +0100, Zhangfei Gao wrote: >>>> Instead of setting hflags in different files, >>>> ahci_platform_get_of_property set hpriv->flags when ahci_platform_init_host >>>> according to property in dts. >>> >>> The AHCI_HFLAGS are a Linux implementation detail, so it would be nice >>> to have a good justification for each of these being turned into DT >>> properties. >> >> Do you mean only add required property now? > > No you should not add any properties at all using properties to set SoC specific quirks > is just wrong, if a device is not 100% ahci compatible you should use a SoC specific > compatible string for it, and set quirks in the kernel driver based on that. > > Devicetree is supposed to describe hardware, in this case the quirks are a property > of the specific model of the hardware, so this should be expressed through the > compatible string. Extra properties in devicetree are meant to be used for board > specific things, like having a gpio to enable power to the sata target device. > Extra properties should not be used in the way you are proposing to use them now. > OK, got it. What I thoutht is using ahci_platform_get_of_property could make ahci_platform.c more shareable, adding the chance of removing specific compatible. Just refer mmc_of_parse and sdhci_get_of_property, they are parsing property directly from dts. Anyway, it is fine, here is not so many cases like mmc. Thanks