Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/ipack/ipack.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
index 7ec6b20..599d4ff 100644
--- a/drivers/ipack/ipack.c
+++ b/drivers/ipack/ipack.c
@@ -24,6 +24,7 @@ static void ipack_device_release(struct device *dev)
{
struct ipack_device *device = to_ipack_dev(dev);
kfree(device->id);
+ device->id = NULL;
device->release(device);
}
--
1.7.10.4
One function is ipack_device_init(). If it fails, the caller should execute
ipack_put_device().
The second function is ipack_device_add that only adds the device. If
it fails, the caller should execute ipack_put_device().
Then the device is removed with refcount = 0, as device_register() kernel
documentation says.
ipack_device_del() is added to remove the device.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/ipack/carriers/tpci200.c | 14 +++++++++++++-
drivers/ipack/ipack.c | 24 +++++++++++++----------
include/linux/ipack.h | 39 ++++++++++++++++++++++++++++----------
3 files changed, 56 insertions(+), 21 deletions(-)
diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index 0246b1f..c276fde 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -480,6 +480,7 @@ static void tpci200_release_device(struct ipack_device *dev)
static int tpci200_create_device(struct tpci200_board *tpci200, int i)
{
+ int ret;
enum ipack_space space;
struct ipack_device *dev =
kzalloc(sizeof(struct ipack_device), GFP_KERNEL);
@@ -495,7 +496,18 @@ static int tpci200_create_device(struct tpci200_board *tpci200, int i)
+ tpci200_space_interval[space] * i;
dev->region[space].size = tpci200_space_size[space];
}
- return ipack_device_register(dev);
+
+ ret = ipack_device_init(dev);
+ if (ret < 0) {
+ ipack_put_device(dev);
+ return ret;
+ }
+
+ ret = ipack_device_add(dev);
+ if (ret < 0)
+ ipack_put_device(dev);
+
+ return ret;
}
static int tpci200_pci_probe(struct pci_dev *pdev,
diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
index bdac7f6..a26f371 100644
--- a/drivers/ipack/ipack.c
+++ b/drivers/ipack/ipack.c
@@ -228,7 +228,7 @@ static int ipack_unregister_bus_member(struct device *dev, void *data)
struct ipack_bus_device *bus = data;
if (idev->bus == bus)
- ipack_device_unregister(idev);
+ ipack_device_del(idev);
return 1;
}
@@ -420,7 +420,7 @@ out:
return ret;
}
-int ipack_device_register(struct ipack_device *dev)
+int ipack_device_init(struct ipack_device *dev)
{
int ret;
@@ -429,6 +429,7 @@ int ipack_device_register(struct ipack_device *dev)
dev->dev.parent = dev->bus->parent;
dev_set_name(&dev->dev,
"ipack-dev.%u.%u", dev->bus->bus_nr, dev->slot);
+ device_initialize(&dev->dev);
if (dev->bus->ops->set_clockrate(dev, 8))
dev_warn(&dev->dev, "failed to switch to 8 MHz operation for reading of device ID.\n");
@@ -448,19 +449,22 @@ int ipack_device_register(struct ipack_device *dev)
dev_err(&dev->dev, "failed to switch to 32 MHz operation.\n");
}
- ret = device_register(&dev->dev);
- if (ret < 0)
- kfree(dev->id);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ipack_device_init);
- return ret;
+int ipack_device_add(struct ipack_device *dev)
+{
+ return device_add(&dev->dev);
}
-EXPORT_SYMBOL_GPL(ipack_device_register);
+EXPORT_SYMBOL_GPL(ipack_device_add);
-void ipack_device_unregister(struct ipack_device *dev)
+void ipack_device_del(struct ipack_device *dev)
{
- device_unregister(&dev->dev);
+ device_del(&dev->dev);
+ ipack_put_device(dev);
}
-EXPORT_SYMBOL_GPL(ipack_device_unregister);
+EXPORT_SYMBOL_GPL(ipack_device_del);
void ipack_get_device(struct ipack_device *dev)
{
diff --git a/include/linux/ipack.h b/include/linux/ipack.h
index def91fd..1888e06 100644
--- a/include/linux/ipack.h
+++ b/include/linux/ipack.h
@@ -207,19 +207,38 @@ int ipack_driver_register(struct ipack_driver *edrv, struct module *owner,
void ipack_driver_unregister(struct ipack_driver *edrv);
/**
- * ipack_device_register -- register an IPack device with the kernel
- * @dev: the new device to register.
+ * ipack_device_init -- initialize an IPack device
+ * @dev: the new device to initialize.
*
- * Register a new IPack device ("module" in IndustryPack jargon). The call
- * is done by the carrier driver. The carrier should populate the fields
- * bus and slot as well as the region array of @dev prior to calling this
- * function. The rest of the fields will be allocated and populated
- * during registration.
+ * Initialize a new IPack device ("module" in IndustryPack jargon). The call
+ * is done by the carrier driver. The carrier should populate the fields
+ * bus and slot as well as the region array of @dev prior to calling this
+ * function. The rest of the fields will be allocated and populated
+ * during initalization.
*
- * Return zero on success or error code on failure.
+ * Return zero on success or error code on failure.
+ *
+ * NOTE: _Never_ directly free @dev after calling this function, even
+ * if it returned an error! Always use ipack_put_device() to give up the
+ * reference initialized in this function instead.
+ */
+int ipack_device_init(struct ipack_device *dev);
+
+/**
+ * ipack_device_add -- Add an IPack device
+ * @dev: the new device to add.
+ *
+ * Add a new IPack device. The call is done by the carrier driver
+ * after calling ipack_device_init().
+ *
+ * Return zero on success or error code on failure.
+ *
+ * NOTE: _Never_ directly free @dev after calling this function, even
+ * if it returned an error! Always use ipack_put_device() to give up the
+ * reference initialized in this function instead.
*/
-int ipack_device_register(struct ipack_device *dev);
-void ipack_device_unregister(struct ipack_device *dev);
+int ipack_device_add(struct ipack_device *dev);
+void ipack_device_del(struct ipack_device *dev);
void ipack_get_device(struct ipack_device *dev);
void ipack_put_device(struct ipack_device *dev);
--
1.7.10.4
Prepare everything for later use.
Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
---
drivers/ipack/ipack.c | 12 ++++++++++++
include/linux/ipack.h | 3 +++
2 files changed, 15 insertions(+)
diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
index 599d4ff..bdac7f6 100644
--- a/drivers/ipack/ipack.c
+++ b/drivers/ipack/ipack.c
@@ -462,6 +462,18 @@ void ipack_device_unregister(struct ipack_device *dev)
}
EXPORT_SYMBOL_GPL(ipack_device_unregister);
+void ipack_get_device(struct ipack_device *dev)
+{
+ get_device(&dev->dev);
+}
+EXPORT_SYMBOL_GPL(ipack_get_device);
+
+void ipack_put_device(struct ipack_device *dev)
+{
+ put_device(&dev->dev);
+}
+EXPORT_SYMBOL_GPL(ipack_put_device);
+
static int __init ipack_init(void)
{
ida_init(&ipack_ida);
diff --git a/include/linux/ipack.h b/include/linux/ipack.h
index fea12cb..def91fd 100644
--- a/include/linux/ipack.h
+++ b/include/linux/ipack.h
@@ -221,6 +221,9 @@ void ipack_driver_unregister(struct ipack_driver *edrv);
int ipack_device_register(struct ipack_device *dev);
void ipack_device_unregister(struct ipack_device *dev);
+void ipack_get_device(struct ipack_device *dev);
+void ipack_put_device(struct ipack_device *dev);
+
/**
* DEFINE_IPACK_DEVICE_TABLE - macro used to describe a IndustryPack table
* @_table: device table name
--
1.7.10.4
On Fri, Mar 08, 2013 at 09:21:45AM +0100, Samuel Iglesias Gonsalvez wrote:
> Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
> ---
> drivers/ipack/ipack.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
> index 7ec6b20..599d4ff 100644
> --- a/drivers/ipack/ipack.c
> +++ b/drivers/ipack/ipack.c
> @@ -24,6 +24,7 @@ static void ipack_device_release(struct device *dev)
> {
> struct ipack_device *device = to_ipack_dev(dev);
> kfree(device->id);
> + device->id = NULL;
How does that keep anything from being freed twice?
> device->release(device);
device should now be gone after this call, right? What am I missing?
thanks,
greg k-h
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 03/08/2013 06:47 PM, Greg Kroah-Hartman wrote:
Yes, you are right. It's not possible to have it freed twice once it's
in ipack_device_release().
You can skip this patch. If you want, I can resend the others accordingly.
Sam
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/
iQIcBAEBCAAGBQJROim2AAoJEH/0ujLxfcND8ksQAIKZJlIONv1h6FWQEzcOrptA
ReEB0p+uDQfRzjpbqzeZcXa3vVK6sc2nIUTggdQcnfNcse38Jn4JCKGllAQfZE51
71ANDmhUpmALKD1o0A8Cya85IfnaVQBhHcmA2b3/wh5dgXjcvgjHWZDjduEC3+I7
sIikZ+OYAZa69dfXYN+NgVYCTj5Gk+eA9oWdnV17d18bviqVf1/PdHHGMceMke9p
jW/UY4mxPHGkJdtZ8RV0avm2VAwY0/HOtzjI8JXZ7Nve9HIYhEcMyBrpHxOQI6U3
CBPY5kqlOa2MdGTiKQct7Daeeqm1guSPPJDRRI3y0O+qgHOn4a8eTVHQ21WMHJoP
Yq1RterlGzRGGzv8s9k76600gXqbaQreEN96fXxISDtk57N/ffIn0cuZwbaeTLpa
AfvV0PeuBpwJ5SezQryNx2O708tk8FGiCys3yAFcak82Am4xBsgIrEo1UYIYXonC
rGbmuG36yxsktMdklz5+ys7R4pDW8Un0iCOMmO3etrYDepy6n9vfjXmYWc8MK2D6
WaVj01qadSj8iHE1cFZms6LnQ+VETc9256QkkFMWaNApW9+BhnGIRj8DErbgOfKK
k1BfmNUrPyuFIlxPxy6kkPwPGfLUOP9yRhGmzrJ18DWJJtdELan5RQIrMlH21LA2
ajvtF3Pt5eDhDy9gM6A8
=LI+5
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 03/08/2013 06:47 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 08, 2013 at 09:21:45AM +0100, Samuel Iglesias
> Gonsalvez wrote:
>> Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
>> --- drivers/ipack/ipack.c | 1 + 1 file changed, 1
>> insertion(+)
>>
>> diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c index
>> 7ec6b20..599d4ff 100644 --- a/drivers/ipack/ipack.c +++
>> b/drivers/ipack/ipack.c @@ -24,6 +24,7 @@ static void
>> ipack_device_release(struct device *dev) { struct ipack_device
>> *device = to_ipack_dev(dev); kfree(device->id); + device->id =
>> NULL;
>
> How does that keep anything from being freed twice?
>
>> device->release(device);
>
> device should now be gone after this call, right? What am I
> missing?
>
Yes, you are right. It's not possible to have it freed twice once it's
in ipack_device_release().
You can skip this patch. If you want, I can resend the others accordingly.
Sam
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/
iQIcBAEBCAAGBQJROim2AAoJEH/0ujLxfcNDY+sQAMfrjjs05FJ9wmQvqEFKg18I
QVddG8BbHKDzdmSTkDlagijTE/Q7eOOFV7zOKz6QWJjO14ktPjr56cFwEmIl0KPj
LcDO4+4bIsOfhO64ZJ7CHUFyMcq3olrZNBhQLJZU1anOcDKIimzomGGPXv5PPFe6
BXnRNeXEnS81r3LXQtlJM7h8th3objKWW2R5No40E9sHIKWEgYR7JmgCqhk0fQNv
RKA9FjyBFpF8RjPoi+xSdPYhFPLpOv99ZbYK1wU4goD4ADw9Npujw/66CjFtgJGx
BH3MquLaCYvCmJK6+obQJzWKl1TonQ153hh6uM1b7h+5RJ4yhUSaSPRLLIWGAxRH
3XlbtjMO6bwEDAEg2hOq9whmoVxK70YvjKOCkqo8crwqVsLccMnv4OzoE4NNrm99
8cdUScK8cTi5fg2HuIEJMVTPxpfLk46Ab3OTO9hoVURyRG84TnJ0SLK0CPzy6J1z
ORgZWh7PlEsgLeqsqjKv7q/5tcLLPEGZcV7TekWPBfNVR5aneemHrEIaSIuVYXxw
a7INgvaAWNhqGhag86KwVH0ICbz2xuJu6wtrlFhyMZWhdPN5aEcHwXusjUBhrdrm
DXoEDBFvJShBE8PxNPWYIBcrphetaSQzl0vNsO/SqacGlppsAqtvvU6hkkAuhGeO
a8dpR7nIR8tuz4NPnFHu
=NypR
-----END PGP SIGNATURE-----
On Fri, Mar 08, 2013 at 07:11:02PM +0100, Samuel Iglesias Gons?lvez wrote:
> On 03/08/2013 06:47 PM, Greg Kroah-Hartman wrote:
> > On Fri, Mar 08, 2013 at 09:21:45AM +0100, Samuel Iglesias
> > Gonsalvez wrote:
> >> Signed-off-by: Samuel Iglesias Gonsalvez <[email protected]>
> >> --- drivers/ipack/ipack.c | 1 + 1 file changed, 1
> >> insertion(+)
> >>
> >> diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c index
> >> 7ec6b20..599d4ff 100644 --- a/drivers/ipack/ipack.c +++
> >> b/drivers/ipack/ipack.c @@ -24,6 +24,7 @@ static void
> >> ipack_device_release(struct device *dev) { struct ipack_device
> >> *device = to_ipack_dev(dev); kfree(device->id); + device->id =
> >> NULL;
> >
> > How does that keep anything from being freed twice?
> >
> >> device->release(device);
> >
> > device should now be gone after this call, right? What am I
> > missing?
> >
>
> Yes, you are right. It's not possible to have it freed twice once it's
> in ipack_device_release().
>
> You can skip this patch. If you want, I can resend the others accordingly.
If the others require this one to be applied, in order for them to apply
properly, yes, it would be great to resend. If not, I'll just skip the
first one.
thanks,
greg k-h
On Fri, Mar 08, 2013 at 11:36:24AM -0800, Greg Kroah-Hartman wrote:
[...]
> If the others require this one to be applied, in order for them to apply
> properly, yes, it would be great to resend. If not, I'll just skip the
> first one.
>
They don't require the first one. You can skip it.
Thanks,
Sam