| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 May 2020 15:17:11 +0800
From: "Ramuthevar, Vadivel MuruganX"
<vadivel.muruganx.ramuthevar@...ux.intel.com>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: tglx@...utronix.de, cheol.yong.kim@...el.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
masonccyang@...c.com.tw, anders.roxell@...aro.org, vigneshr@...com,
arnd@...db.de, hauke.mehrtens@...el.com, richard@....at,
brendanhiggins@...gle.com, linux-mips@...r.kernel.org,
robh+dt@...nel.org, linux-mtd@...ts.infradead.org,
miquel.raynal@...tlin.com, qi-ming.wu@...el.com,
andriy.shevchenko@...el.com
Subject: Re: [PATCH v4 2/2] mtd: rawnand: Add NAND controller support on Intel
LGM SoC
Hi Boris,
On 5/5/2020 3:00 pm, Boris Brezillon wrote:
> Hello Vadivel,
>
> On Tue, 5 May 2020 13:28:58 +0800
> "Ramuthevar, Vadivel MuruganX"
> <vadivel.muruganx.ramuthevar@...ux.intel.com> wrote:
>
>>>>>>>>
>>>>>>>> ebu_nand: ebu_nand@...00000 {
>>>>>>>> compatible = "intel,lgm-ebu-nand";
>>>>>>>> reg = <0xe0f00000 0x100
>>>>>>>> 0xe1000000 0x300
>>>>>>>> 0xe1400000 0x80000
>>>>>>>> 0xe1c00000 0x10000>;
>>>>>>>> reg-names = "ebunand", "hsnand", "nand_cs0", nand_cs1";
>>>>>>>> dmas = <&dma0 8>, <&dma0 9>;
>>>>>>>> dma-names = "ebu_rx", "ebu_tx";
>>>>>>>> clocks = <&cgu0 LGM_GCLK_EBU>;
>>>>>>>> };
>>>>>>>>
>>>>>>>>
>>>>>>>> &ebu_nand {
>>>>>>>> status = "disabled";
>>>>>>>> nand,cs = <1>;
>>>>>>>> nand-ecc-mode = "hw";
>>>>>>>> pinctrl-names = "default";
>>>>>>>> pinctrl-0 = <&ebu_nand_base &ebu_cs1>;
>>>>>>>> };
>>>>>>>>
>>>>>>>>>
>>>>>>> Ok. If I understand the SoC topology correctly it should actually be
>>>>>>> something like that:
>>>>>>>
>>>>>>> {
>>>>>>> ...
>>>>>>> fpi@...xx {
>>>>>>> compatible = "intel,lgm-fpi", "simple-bus";
>>>>>>>
>>>>>>> /* You might have other ranges to define here */
>>>>>>> ranges = <0x16000000 0xe0000000 0x1000000>;
>>>>>>>
>>>>>>> ...
>>>>>>
>>>>>> Sorry, we do not have fpi tree node in our dts/dtsi file instead we have
>>>>>> the below one.. , that also not included the major peripherals
>>>>>> controllers node.
>>>>>> /* Special part from CPU core */
>>>>>> core: core {
>>>>>> compatible = "intel,core", "simple-bus";
>>>>>> #address-cells = <1>;
>>>>>> #size-cells = <1>;
>>>>>> ranges;
>>>>>>
>>>>>> ioapic1: interrupt-controller@...00000 {
>>>>>> #interrupt-cells = <2>;
>>>>>> #address-cells = <0>;
>>>>>> compatible = "intel,ce4100-ioapic";
>>>>>> interrupt-controller;
>>>>>> reg = <0xfec00000 0x1000>;
>>>>>> nr_entries = <256>;
>>>>>> };
>>>>>>
>>>>>> hpet: timer@...00000 {
>>>>>> compatible = "intel,ce4100-hpet";
>>>>>> reg = <0xfed00000 0x400>;
>>>>>> };
>>>>>>
>>>>>> lapic0: interrupt-controller@...00000 {
>>>>>> compatible = "intel,ce4100-lapic";
>>>>>> reg = <0xfee00000 0x1000>;
>>>>>> no_pic_mode;
>>>>>> };
>>>>>> };
>>>>>>
>>>>>> other than this, rest all in independent node .
>>>>>
>>>>> But you do have an FPI bus, right? If this is the case it should be
>>>>> represented.
>>>>
>>>> Yes, FPI bus is slave to core which connects all the peripherals.
>>>>
>>>> Or is the "intel,core" bus actually the FPI bus that you
>>>>> named differently?
>>>>
>>>> FPI slave bus connects to core bus by OCP bridge, so here it is named
>>>> FPI bus, but SW perspective didn't have root tree which has all
>>>> sub-nodes, as of now each peripheral has its own node.
>>>
>>> Duh, not sure that's a good idea to hide that, especially since you
>>> have to describe the address translation that happens when crossing the
>>> FPI bus (the ranges thing I mentioned previously).
>>
>> Thanks! for the keep reviewing.
>>
>> SW Address translation is not required, after discussion with HW team ,
>> came to know that 0x17400 and 0x17C00 internal to the SoC.
>>
>> NOC will translate 0xE1XX... to FPI address 0x17X... internally.
>> There is an address translation in the NOC.
>> 0x17X... is not visible to user.
>>
>> so far added hard-coded values to CS0 and CS1 is not at required.
>> I will change the code accordingly and sent to you.
>
> Hm, you told me last week that writing wrong values to this register
> caused the NAND controller to not work properly (you even had code that
> was overwriting the dynamically calculated values by hardcoded ones, so
> I suspect it indeed didn't work) and now you say this write to
> EBU_ADDR_SEL() is optional?! Sorry but it's hard to believe, and I've
> received so many contradictory information from you on that matter that
> I can't really tell which one is correct. Not sure I want to keep
> reviewing new versions of this driver in this context.
oh my bad really sorry , since last week based on the input given from
person who has worked on legacy IP, but now I have discussed with
low-level HW team and confirmed. also we don't have proper document
reference manual since it's new SoC.
Please forgive me , this shouldn't be happen once again, Thanks a lot!!
Regards
Vadivel
>
> Regards,
>
> Boris
>
Powered by blists - more mailing lists