Hello.

On Mon, 2008-07-07 at 21:21, pHilipp Zabel wrote:
quoted text
> On Mon, Jul 7, 2008 at 8:40 PM,  <stefan@datenfreihafen.org> wrote:
> > The PCAP Asic as present on EZX phones is a multi function device with voltage
> > regulators, irq expander, touch screen controller and audio codec.
> > It is connected to the processor via SPI, this driver provides read/write
> > functions to its registers and a irq demultiplexer.
> 
> Are the EZX phones expected to be the only linux devices with PCAP?

That is somewhat hard to predict as Motorola was not interested in helping us
with the driver for mainline and we never got datasheets or technical support.
From what we have researched it seems that they use an antecessor of this chip
in older feature phones. As far as we know none of them runs linux.

quoted text
> Otherwise I'd dislike the hardcoded IRQ numbers in the MFD driver.

So far it seems to be no problem. If there is a strong feeling to change this we
will of course work on that. Personally I would prefer to do this when new users
of this driver appear.

quoted text
> The machine_is_xyz() calls inside ezx-pcap could be replaced with
> configuration via platform_data (have pcap_platform_data->cs_inverted,
> for example)

We had this before and it turned out as messy as this solution. We have one
machine file that supports up to 6 devices right now. So we would have to deal
with the same machine_is_xyz macros in the machine file.

We thought it would be better to let the driver handle this. Open for discussion
of course.

quoted text
> > +#if 0
> > +#define DEBUGP(x, args...) printk(x, ## args)
> > +#else
> > +#define DEBUGP(x, args...)
> > +#endif
> 
> Could you remove the custom debug macros and use pr_debug (or even
> better, dev_dbg) instead?

Will do.

quoted text
> > +static void __exit ezx_pcap_exit(void)
> > +{
> > +       return platform_driver_unregister(&ezxpcap_driver);
> > +}
> > +
> > +module_init(ezx_pcap_init);
> 
> Depending on what platform_devices depend on this, maybe use
> subsys_initcall here.

Ok, I'll need to look up on this to see if it is an option.

quoted text
> > +module_exit(ezx_pcap_exit);
> 
> Why bother with module_exit when the Kconfig option is bool?

Well, matter of taste. :)

Taking care about cleanup is good style. Still you are right that it is useles
for the time being.

quoted text
> > +#define PCAP_IRQ_USB4V         (1 << 6)        /* USB above 4volt???
> > +                                               called "USBDET_4V" in blob */
> 
> I assume that's for OTG operation. The VBUS voltage is valid from 4.4 V, and the
> PXA27x UDC controller has "Vbus valid 4.0 V" and "Vbus valid 4.4 V" interrupts

ok

quoted text
> > +       /*  set core voltage */
> > +       ezx_pcap_set_sw(SW1, SW_VOLTAGE, SW_VOLTAGE_1250);
> 
> Btw, did you see the voltage regulator framework that is in linux-next?

Heard of it, but not read the code yet. It's on my list. I just a bit hesitant
to base on code that is not in mainline yet. But -next should be ok.

quoted text
> > -
> > +       platform_device_register(&ezx_pcap_device);
> >        platform_add_devices(devices, ARRAY_SIZE(devices));
> 
> You could put ezx_pcap_device into the beginning of devices[].

Duh, will fix.

I send out an updated patch tomorrow. Need some sleep now.

Thanks for the feedback.

regards
Stefan Schmidt
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/