Re: [PATCH v4 3/3] arm64: dts: mt8173: Add nor flash node

bayi.cheng
2015-10-14T13:59:29Z

On Wed, 2015-10-14 at 19:26 +0800, Daniel Kurtz wrote:
> Sergei,
>
> On Wed, Oct 14, 2015 at 7:05 PM, Sergei Shtylyov
> <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> > Hello.
> >
> > On 10/14/2015 4:07 AM, Daniel Kurtz wrote:
> >
> >>>> Add Mediatek nor flash node
> >>>>
> >>>> Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>> ---
> >>>> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 15 +++++++++++++++
> >>>> 1 file changed, 15 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>>> b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>>> index d18ee42..385c2e4 100644
> >>>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> >>>> @@ -365,6 +365,21 @@
> >>>> status = "disabled";
> >>>> };
> >>>>
> >>>> + nor_flash: spi@1100d000 {
> >>>
> >>>
> >>> I don't think this is a good label name for a SPI controller.
> >>
> >>
> >> You think "spi" is not a good name for SPI controller?
> >> Or you mean the label "nor_flash"?
> >
> >
> > I think I said clearly: "label". :-)
> >
> >> Would you prefer:
> >>
> >> spi_nor_flash: spi@1100d000
> >
> >
> > I'd prefer the "_nor_flash" part to be dropped, to be used for the real
> > flash device (if needed) which is a sub-node of this node.
>
> The dedicated nor_flash spi bus cannot just be labeled 'spi', as that
> would conflict with the general purpose 'spi' bus. Instead, it is
> given a unique label that can be used as a phandle elsewhere (e.g., in
> a board file, to enable the bus). The name "spi_nor_flash" makes it
> clear that this spi bus is only for use for accessing the nor_flash.
>
> Bayi: Actually, this reminds me. I'd prefer if the bus node was
> disabled by default, and only enabled by boards as required. So, the
> node should include:
> status = "disabled";
>
> -Dan

Hi, Dan & Sergei, Thanks for your advice, and I will add "status =
"disable"" to the node in the next patch.Thanks~

Bayi