Re: [PATCH] pxamci: remove an ifdef about CONFIG_REGULATOR
On Tue, May 10, 2011 at 10:02:14PM +0200, Antonio Ospite wrote: I was blindly trusting code already in mainline again, and for that I apologize, I finally took the time to look at the implementation of IS_ERR() and test its use, and being IS_ERR(NULL) true it is not what we want indeed, see the attached test program. So, I am going to remove the ifdefs anyway but use IS_ERR_OR_NULL(); how does that sound? Am I still missing anything? That sounds a lot better, and should avoid the issue which caused me to throw out the original patch.
Re: Using statically allocated memory for platform_data.
On Mon, Nov 02, 2009 at 03:00:11PM +, Ben Dooks wrote: This looks like something is freeing stuff that it did not allocate in the first place, which is IMHO bad. The call platform_device_alloc() is setting platform_device_release() as the default release function but platform_device_release() releases more than platform_device_alloc() actually created. My view is that platform_device_alloc()'s default release shouldn't be freeing the platform data, and that using platform_device_add_data() or platform_device_add_resources() should change either the behvaiour of platform_device_release() or it should change the pointer to a new release function. That doesn't work - how do those other functions (adding) know what data has also been added by other functions? That can't work reliably.
Re: Using statically allocated memory for platform_data.
On Mon, Nov 02, 2009 at 04:28:39PM +, Ben Dooks wrote: On Mon, Nov 02, 2009 at 03:56:25PM +, Russell King - ARM Linux wrote: The reason we have platform_device_add_data() is that people think that the device data needs to persist for the lifetime of the device. I personally disagree with that - once you unregister the device, it's guaranteed that device drivers will have been unregistered, so who's going to use the platform data? That doesn't make any sense, in the current case of using the platform_device_alloc() and those calls the data is only living for the lifetime of the device, as the release call is tidying up the result. What I'm saying is that the lifetime of the data finishes once the _unregister() call has returned. So: data = pdev-dev.platform_data; platform_device_unregister(pdev); kfree(data); is an entirely valid way of handling the I allocated my platform data problem - it doesn't need to exist to the point where the device itself is freed. There are a number of places where this data isn't __initdata, and still needs to be copied, and then freed once the device has been removed. What I'm saying is that the point where the platform data can be freed is the point where the device is unregistered. It is not the point where the device is actually freed. Whenever you have the pointer for the platform device to be unregistered, you also have the pointer for its data available. If we accept that the lifetime for the platform data is from the point the device is registered to the point immediately following when the device is unregistered, then the above solution is acceptable and we don't need to play games with release pointers.
Re: [WARNING] pxamci: 'pxa2xx-mci.0' does not have a release() function.
On Fri, Oct 23, 2009 at 12:36:31AM +0200, Antonio Ospite wrote: Hi, I get this warning on shutdown. How to fix it properly? FYI, in my setup pxa2xx_spi is the parent for pxamci, set using this patch: http://git.openezx.org/openezx.git?a=commitdiff;h=0ffd85ad8faea3456d4ecf5f63ae65aca26fff21 This sounds like it's the cause of the problem - from the backtrace, it looks like SPI expects the children of the SPI device to be its own responsibility to maintain. Hence, because you've made pxamci a child of SPI, SPI is trying to unregister and release the pxamci device.
Re: [PATCH v4] pxamci: add regulator support.
On Thu, May 14, 2009 at 05:11:00PM +0800, Eric Miao wrote:
+static void pxamci_set_power(struct pxamci_host *host, unsigned int vdd)
{
...
+ struct mmc_host *mmc = host-mmc;
...
+ if (host-pdata-setpower) {
+ host-pdata-setpower(host-dev, vdd);
+ return;
+ }
...
+#ifdef CONFIG_REGULATOR
+ /* use regulator API */
+ if (!host-vcc) {
+ host-vcc = regulator_get(dev, vmmc);
+ if (IS_ERR(host-vcc))
+ return PTR_ERR(host-vcc);
...
+ mmc-ocr_avail = mmc_regulator_get_ocrmask(host-vcc);
+ }
This is really not a good idea. ocr_avail is expected to be set at
initialization time and should remain a constant. It is not expected
to change.
Re: [PATCH/RFC] pxa_set_mci_parent
On Wed, Apr 08, 2009 at 11:20:25AM -0300, Daniel Ribeiro wrote: The card may be powered by a SPI or I2C connected PMIC or voltage regulator. This patch provides a set_mci_parent function to assert proper suspend/resume ordering. It would be better to roll this into pxa_set_mci_info() so that there isn't a possibility of registering the device and then (illegally) changing the parent.
Re: [PATCH/RFC] pxa_set_udc_parent
On Wed, Apr 08, 2009 at 11:24:00AM -0300, Daniel Ribeiro wrote: The UDC may be connected to an external USB transceiver on SPI or I2c, this patch allows setting the pxa2xx_udc parent to assert proper suspend/resume ordering. This could well break platforms - requiring them to call pxa_set_udc_info() to have the UDC device registered. If you remove a device from the generic device arrays, you need to ensure that all platforms currently merged still register the device.
