linux-kernel - Re: [PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers in DT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110825145736.GA31331@huya.qualcomm.com>
Date:	Thu, 25 Aug 2011 07:57:36 -0700
From:	David Brown <davidb@...eaurora.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	David Brown <davidb@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	Daniel Walker <dwalker@...o99.com>,
	Bryan Huntsman <bryanh@...eaurora.org>,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Stephen Boyd <sboyd@...eaurora.org>
Subject: Re: [PATCH v3 4/4] ARM: msm: Describe MSM 8660 SURF FPGA registers
 in DT

On Thu, Aug 25, 2011 at 01:27:12PM +0200, Arnd Bergmann wrote:
> On Thursday 18 August 2011, David Brown wrote:
> > +static void __init msm8660_surf_fpga_init(void __iomem *fpga_mem)
> > +{
> > +       /* Advanced mode */
> > +       writew(0xFFFF, fpga_mem + 0x15C);
> > +       /* FPGA_UART_SEL */
> > +       writew(0, fpga_mem + 0x172);
> > +       /* FPGA_GPIO_CONFIG_117 */
> > +       writew(1, fpga_mem + 0xEA);
> > +       /* FPGA_GPIO_CONFIG_118 */
> > +       writew(1, fpga_mem + 0xEC);
> > +       dmb();
> > +}
> 
> Does the dmb() do the right thing here? It seems strange to combine a strictly
> ordered I/O instruction with another ordering instruction, and I think it would
> be better to use writew_relaxed for the first one, followed by a 'wmb()'.

I guess I didn't really think about that, I just kind of kept the
code.  I'll ask Stepan why he did it that way, and come up with a
cleaner solution.

> > +#ifdef CONFIG_OF
> > +static void __init msm8660_surf_fpga_init_dt(void)
> > +{
> > +       struct device_node *node;
> > +       void __iomem *fpga_mem;
> > +
> > +       node = of_find_compatible_node(NULL, NULL, "qcom,msm8660-surf-fpga");
> > +       if (!node)
> > +               return;
> > +
> > +       fpga_mem = of_iomap(node, 0);
> > +       of_node_put(node);
> > +       if (!fpga_mem) {
> > +               printk(KERN_ERR "%s: Can't map fpga registers\n", __func__);
> > +               return;
> > +       }
> > +
> > +       msm8660_surf_fpga_init(fpga_mem);
> > +       iounmap(fpga_mem);
> > +}
> > +#endif
> 
> Is the serial port connected through the FPGA or just configured by it?

The FPGA controls how the UART pins are connected on the development
board.  The serial port itself is in the MSM, not the FPGA, and on
other dev boards this isn't needed for the serial port to work.

> In the former case, I think it would be better to make this a proper
> device driver that binds to the qcom,msm8660-surf-fpga device,
> configures it and then creates the platform_devices for the child
> nodes (the serial port, possibly others) by calling
> of_platform_bus_probe.

It might make sense to have the FPGA as a driver.  I believe it was
done early to make sure that the pins were configured correctly before
the serial driver came up.  As far as I can tell, the output pin is
already configured correctly, so this can actually happen completely
independently, since early usage of the UART is really only for
console messages.

I don't think it makes sense for the serial to be a child node, this
FPGA configuration is more along the lines of pinmux.  Most
configurations of this SOC don't have or need this fpga.

So, if I made it a separate driver, where would it go?  Since this
board still has platform device support, I suspect the platform data
needed to describe this device would end up being larger than the
driver itself.

David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/