On Thursday 25 August 2011, David Brown wrote:
> 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.
Yes, no worries. I saw later that the code already exists in similar form,
so it is not urgent to change.
> > > +#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.
ok.
> > 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.
Agreed.
> 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.
Excellent question ;-)
When the driver is really small, I would just leave it in the board
file for now, although that might not be a good long-term strategy.
Do we have any similar cases that we can group together with the
fpga to make a subsystem? Maybe it could be a small driver in the
pinmux subsystem when that is established.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
