All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: How to get driver_data of struct ieee1394_device_id in kernel driver module?
       [not found] <51A20AEE.7060201@sakamocchi.jp>
@ 2013-05-26 21:35 ` Stefan Richter
  2013-05-26 22:15   ` Stefan Richter
  2013-05-26 22:57   ` Greg Kroah-Hartman
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Richter @ 2013-05-26 21:35 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: linux1394-devel, linux-kernel, Greg Kroah-Hartman

(Adding LKML and Greg KH, in case that there are general opinions about
how a bus_type should work.)

On May 26 Takashi Sakamoto wrote:
> Hi all,
> 
> I'm a newbile for Linux Firewire subsystem and not used to IEEE 1394 bus
> programing in Linux.
> So I happy to get your advices.
> 
> Currently I'm working for some drivers of ALSA in kernel land.
> Then some devices to which the same module applies need to handle its
> own operations.
> To implement these operations, I think it good to utilize driver_data of
> struct ieee1394_evice_id entry.
> 
> This is example.
> For entry:
> https://github.com/takaswie/snd-firewire-improve/blob/f509e4587c3f7b18eda70d07b616ef01be3546ef/bebob/bebob.c#L462
> For usage:
> https://github.com/takaswie/snd-firewire-improve/blob/f509e4587c3f7b18eda70d07b616ef01be3546ef/bebob/bebob.c#L362
> 
> (In this case, I use the cast between (kernel_ulong_t) and (struct
> snd_bebob_ops *) but I have no confidence that this is better...)
> 
> Anyway, this module need to know the id for the current device in
> device_id table but I have no idea to get the id. Do you know good way
> to get it in module's probing process?
> 
> 
> Regards
> 
> Takashi Sakamoto

I think your approach is sensible.  There is of course just the little
problem that firewire-core keeps the matching device_id table entry as a
secret to itself.  Therefore, struct ieee1394_device_id.driver_data is
currently totally useless.

How about we make it available like in the following patch?

Besides being useful to your presently out-of-tree work, the in-tree
sound/firewire/speakers.c::fwspk_detect() could be rewritten to use this
approach.  Maybe I will post an expanded version of this patch which
incorporates such a first in-tree usage.

-------- 8< --------

From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: [PATCH] firewire: pass matching ieee1394_device_id to drivers via struct fw_unit

FireWire audio unit drivers will need to apply various model-specific
operations.  The struct ieee1394_device_id.driver_data member can be
used to store respective flags or function pointer tables.

But until now drivers had no way to know which driver->id_table index
was matched with an fw_unit instance.  Other bus subsystems like pci or
usb pass this information via an own driver probe method, whereas the
firewire subsystem uses the stock struct device_driver.probe().

We could introduce a struct fw_driver.probe() which works similarly to
struct pci_driver.probe() or struct usb_driver.probe().  But it seems
simpler to just add a new member to struct fw_unit which caches the
matching ieee1394_device_id, so this is what is done here.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-device.c |    7 +++++--
 include/linux/firewire.h       |    5 +++--
 2 files changed, 8 insertions(+), 4 deletions(-)

--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -169,6 +169,7 @@ static bool is_fw_unit(struct device *de
 
 static int fw_unit_match(struct device *dev, struct device_driver *drv)
 {
+	struct fw_unit *unit = fw_unit(dev);
 	const struct ieee1394_device_id *id_table =
 			container_of(drv, struct fw_driver, driver)->id_table;
 	int id[] = {0, 0, 0, 0};
@@ -177,11 +178,13 @@ static int fw_unit_match(struct device *
 	if (!is_fw_unit(dev))
 		return 0;
 
-	get_modalias_ids(fw_unit(dev), id);
+	get_modalias_ids(unit, id);
 
 	for (; id_table->match_flags != 0; id_table++)
-		if (match_ids(id_table, id))
+		if (match_ids(id_table, id)) {
+			unit->device_id = id_table;
 			return 1;
+		}
 
 	return 0;
 }
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -216,12 +216,15 @@ static inline int fw_device_is_shutdown(
 
 int fw_device_enable_phys_dma(struct fw_device *device);
 
+struct ieee1394_device_id;
+
 /*
  * fw_unit.directory must not be accessed after device_del(&fw_unit.device).
  */
 struct fw_unit {
 	struct device device;
 	const u32 *directory;
+	const struct ieee1394_device_id *device_id;
 	struct fw_attribute_group attribute_group;
 };
 
@@ -247,8 +250,6 @@ static inline struct fw_device *fw_paren
 	return fw_device(unit->device.parent);
 }
 
-struct ieee1394_device_id;
-
 struct fw_driver {
 	struct device_driver driver;
 	/* Called when the parent device sits through a bus reset. */


-- 
Stefan Richter
-=====-===-= -=-= ==-=-
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to get driver_data of struct ieee1394_device_id in kernel driver module?
  2013-05-26 21:35 ` How to get driver_data of struct ieee1394_device_id in kernel driver module? Stefan Richter
@ 2013-05-26 22:15   ` Stefan Richter
  2013-05-26 22:57   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Richter @ 2013-05-26 22:15 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Takashi Sakamoto, linux1394-devel, linux-kernel,
	Greg Kroah-Hartman, Clemens Ladisch

On May 26 Stefan Richter wrote:
> (Adding LKML and Greg KH, in case that there are general opinions about
> how a bus_type should work.)
> 
> On May 26 Takashi Sakamoto wrote:
> > Hi all,
> > 
> > I'm a newbile for Linux Firewire subsystem and not used to IEEE 1394 bus
> > programing in Linux.
> > So I happy to get your advices.
> > 
> > Currently I'm working for some drivers of ALSA in kernel land.
> > Then some devices to which the same module applies need to handle its
> > own operations.
> > To implement these operations, I think it good to utilize driver_data of
> > struct ieee1394_evice_id entry.
> > 
> > This is example.
> > For entry:
> > https://github.com/takaswie/snd-firewire-improve/blob/f509e4587c3f7b18eda70d07b616ef01be3546ef/bebob/bebob.c#L462
> > For usage:
> > https://github.com/takaswie/snd-firewire-improve/blob/f509e4587c3f7b18eda70d07b616ef01be3546ef/bebob/bebob.c#L362
> > 
> > (In this case, I use the cast between (kernel_ulong_t) and (struct
> > snd_bebob_ops *) but I have no confidence that this is better...)
> > 
> > Anyway, this module need to know the id for the current device in
> > device_id table but I have no idea to get the id. Do you know good way
> > to get it in module's probing process?
> > 
> > 
> > Regards
> > 
> > Takashi Sakamoto
> 
> I think your approach is sensible.  There is of course just the little
> problem that firewire-core keeps the matching device_id table entry as a
> secret to itself.  Therefore, struct ieee1394_device_id.driver_data is
> currently totally useless.
> 
> How about we make it available like in the following patch?


From: Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: firewire: pass matching ieee1394_device_id to drivers via struct fw_unit

FireWire audio unit drivers will need to apply various model-specific
operations.  The struct ieee1394_device_id.driver_data member can be
used to store respective flags or function pointer tables.

But until now drivers had no way to know which driver->id_table index
was matched with an fw_unit instance.  Other bus subsystems like pci or
usb pass this information via an own driver probe method, whereas the
firewire subsystem uses the stock struct device_driver.probe().

We could introduce a struct fw_driver.probe() which works similarly to
struct pci_driver.probe() or struct usb_driver.probe().  But it seems
simpler to just add a new member to struct fw_unit which caches the
matching ieee1394_device_id, so this is what is done here, and used in
the snd-firewire-speakers driver.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-device.c |    7 +++-
 include/linux/firewire.h       |    5 ++-
 sound/firewire/speakers.c      |   65 ++++++++++++---------------------
 3 files changed, 32 insertions(+), 45 deletions(-)

--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -168,21 +168,24 @@ static bool match_ids(const struct ieee1
 static bool is_fw_unit(struct device *dev);
 
 static int fw_unit_match(struct device *dev, struct device_driver *drv)
 {
+	struct fw_unit *unit = fw_unit(dev);
 	const struct ieee1394_device_id *id_table =
 			container_of(drv, struct fw_driver, driver)->id_table;
 	int id[] = {0, 0, 0, 0};
 
 	/* We only allow binding to fw_units. */
 	if (!is_fw_unit(dev))
 		return 0;
 
-	get_modalias_ids(fw_unit(dev), id);
+	get_modalias_ids(unit, id);
 
 	for (; id_table->match_flags != 0; id_table++)
-		if (match_ids(id_table, id))
+		if (match_ids(id_table, id)) {
+			unit->device_id = id_table;
 			return 1;
+		}
 
 	return 0;
 }
 
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -215,14 +215,17 @@ static inline int fw_device_is_shutdown(
 }
 
 int fw_device_enable_phys_dma(struct fw_device *device);
 
+struct ieee1394_device_id;
+
 /*
  * fw_unit.directory must not be accessed after device_del(&fw_unit.device).
  */
 struct fw_unit {
 	struct device device;
 	const u32 *directory;
+	const struct ieee1394_device_id *device_id;
 	struct fw_attribute_group attribute_group;
 };
 
 static inline struct fw_unit *fw_unit(struct device *dev)
@@ -246,10 +249,8 @@ static inline struct fw_device *fw_paren
 {
 	return fw_device(unit->device.parent);
 }
 
-struct ieee1394_device_id;
-
 struct fw_driver {
 	struct device_driver driver;
 	/* Called when the parent device sits through a bus reset. */
 	void (*update)(struct fw_unit *unit);
--- a/sound/firewire/speakers.c
+++ b/sound/firewire/speakers.c
@@ -657,44 +657,8 @@ static void fwspk_card_free(struct snd_c
 	fw_unit_put(fwspk->unit);
 	mutex_destroy(&fwspk->mutex);
 }
 
-static const struct device_info *fwspk_detect(struct fw_device *dev)
-{
-	static const struct device_info griffin_firewave = {
-		.driver_name = "FireWave",
-		.short_name  = "FireWave",
-		.long_name   = "Griffin FireWave Surround",
-		.pcm_constraints = firewave_constraints,
-		.mixer_channels = 6,
-		.mute_fb_id   = 0x01,
-		.volume_fb_id = 0x02,
-	};
-	static const struct device_info lacie_speakers = {
-		.driver_name = "FWSpeakers",
-		.short_name  = "FireWire Speakers",
-		.long_name   = "LaCie FireWire Speakers",
-		.pcm_constraints = lacie_speakers_constraints,
-		.mixer_channels = 1,
-		.mute_fb_id   = 0x01,
-		.volume_fb_id = 0x01,
-	};
-	struct fw_csr_iterator i;
-	int key, value;
-
-	fw_csr_iterator_init(&i, dev->config_rom);
-	while (fw_csr_iterator_next(&i, &key, &value))
-		if (key == CSR_VENDOR)
-			switch (value) {
-			case VENDOR_GRIFFIN:
-				return &griffin_firewave;
-			case VENDOR_LACIE:
-				return &lacie_speakers;
-			}
-
-	return NULL;
-}
-
 static int fwspk_probe(struct device *unit_dev)
 {
 	struct fw_unit *unit = fw_unit(unit_dev);
 	struct fw_device *fw_dev = fw_parent_device(unit);
@@ -711,13 +675,10 @@ static int fwspk_probe(struct device *un
 	fwspk = card->private_data;
 	fwspk->card = card;
 	mutex_init(&fwspk->mutex);
 	fwspk->unit = fw_unit_get(unit);
-	fwspk->device_info = fwspk_detect(fw_dev);
-	if (!fwspk->device_info) {
-		err = -ENODEV;
-		goto err_unit;
-	}
+	fwspk->device_info =
+		(const struct device_info *)unit->device_id->driver_data;
 
 	err = cmp_connection_init(&fwspk->connection, unit, 0);
 	if (err < 0)
 		goto err_unit;
@@ -797,8 +758,28 @@ static void fwspk_bus_reset(struct fw_un
 
 	amdtp_stream_update(&fwspk->stream);
 }
 
+static const struct device_info griffin_firewave = {
+	.driver_name = "FireWave",
+	.short_name  = "FireWave",
+	.long_name   = "Griffin FireWave Surround",
+	.pcm_constraints = firewave_constraints,
+	.mixer_channels = 6,
+	.mute_fb_id   = 0x01,
+	.volume_fb_id = 0x02,
+};
+
+static const struct device_info lacie_speakers = {
+	.driver_name = "FWSpeakers",
+	.short_name  = "FireWire Speakers",
+	.long_name   = "LaCie FireWire Speakers",
+	.pcm_constraints = lacie_speakers_constraints,
+	.mixer_channels = 1,
+	.mute_fb_id   = 0x01,
+	.volume_fb_id = 0x01,
+};
+
 static const struct ieee1394_device_id fwspk_id_table[] = {
 	{
 		.match_flags  = IEEE1394_MATCH_VENDOR_ID |
 				IEEE1394_MATCH_MODEL_ID |
@@ -807,8 +788,9 @@ static const struct ieee1394_device_id f
 		.vendor_id    = VENDOR_GRIFFIN,
 		.model_id     = 0x00f970,
 		.specifier_id = SPECIFIER_1394TA,
 		.version      = VERSION_AVC,
+		.driver_data  = (kernel_ulong_t)&griffin_firewave,
 	},
 	{
 		.match_flags  = IEEE1394_MATCH_VENDOR_ID |
 				IEEE1394_MATCH_MODEL_ID |
@@ -817,8 +799,9 @@ static const struct ieee1394_device_id f
 		.vendor_id    = VENDOR_LACIE,
 		.model_id     = 0x00f970,
 		.specifier_id = SPECIFIER_1394TA,
 		.version      = VERSION_AVC,
+		.driver_data  = (kernel_ulong_t)&lacie_speakers,
 	},
 	{ }
 };
 MODULE_DEVICE_TABLE(ieee1394, fwspk_id_table);


-- 
Stefan Richter
-=====-===-= -=-= ==-==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to get driver_data of struct ieee1394_device_id in kernel driver module?
  2013-05-26 21:35 ` How to get driver_data of struct ieee1394_device_id in kernel driver module? Stefan Richter
  2013-05-26 22:15   ` Stefan Richter
@ 2013-05-26 22:57   ` Greg Kroah-Hartman
  2013-05-29 18:09     ` Stefan Richter
  2013-05-30 12:14     ` Takashi Sakamoto
  1 sibling, 2 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2013-05-26 22:57 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Takashi Sakamoto, linux1394-devel, linux-kernel

On Sun, May 26, 2013 at 11:35:13PM +0200, Stefan Richter wrote:
> I think your approach is sensible.  There is of course just the little
> problem that firewire-core keeps the matching device_id table entry as a
> secret to itself.  Therefore, struct ieee1394_device_id.driver_data is
> currently totally useless.
> 
> How about we make it available like in the following patch?
> 
> Besides being useful to your presently out-of-tree work, the in-tree
> sound/firewire/speakers.c::fwspk_detect() could be rewritten to use this
> approach.  Maybe I will post an expanded version of this patch which
> incorporates such a first in-tree usage.

Why not pass it in the probe() function, like USB and PCI does?  That
way, if the driver wants to save it for that device, it can.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to get driver_data of struct ieee1394_device_id in kernel driver module?
  2013-05-26 22:57   ` Greg Kroah-Hartman
@ 2013-05-29 18:09     ` Stefan Richter
  2013-05-30 12:14     ` Takashi Sakamoto
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Richter @ 2013-05-29 18:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Takashi Sakamoto, linux1394-devel, linux-kernel

On May 27 Greg Kroah-Hartman wrote:
> Why not pass it in the probe() function, like USB and PCI does?  That
> way, if the driver wants to save it for that device, it can.

I will try that, probably at the weekend.
-- 
Stefan Richter
-=====-===-= -=-= ===-=
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to get driver_data of struct ieee1394_device_id in kernel driver module?
  2013-05-26 22:57   ` Greg Kroah-Hartman
  2013-05-29 18:09     ` Stefan Richter
@ 2013-05-30 12:14     ` Takashi Sakamoto
  2013-06-02 22:27       ` [PATCH] firewire: introduce fw_driver.probe and .remove methods Stefan Richter
  1 sibling, 1 reply; 12+ messages in thread
From: Takashi Sakamoto @ 2013-05-30 12:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Stefan Richter; +Cc: linux1394-devel, linux-kernel

Hi Stefan and Greg,

I'm glad to contact with you.

I'm quite a beginner of Linux Firewire subsystem (Juju) and the others 
like PCI and USB. So it's hard for me to realize what Juju should be.
But I can rebuild Juju and test it with my devices and modules for which 
I'm working. I'm pleased to test your patches in this way if you need to 
check probe() function in driver module.


Thanks

Takashi Sakamoto

(May 27 2013 07:57), Greg Kroah-Hartman wrote:
> On Sun, May 26, 2013 at 11:35:13PM +0200, Stefan Richter wrote:
>> I think your approach is sensible.  There is of course just the little
>> problem that firewire-core keeps the matching device_id table entry as a
>> secret to itself.  Therefore, struct ieee1394_device_id.driver_data is
>> currently totally useless.
>>
>> How about we make it available like in the following patch?
>>
>> Besides being useful to your presently out-of-tree work, the in-tree
>> sound/firewire/speakers.c::fwspk_detect() could be rewritten to use this
>> approach.  Maybe I will post an expanded version of this patch which
>> incorporates such a first in-tree usage.
>
> Why not pass it in the probe() function, like USB and PCI does?  That
> way, if the driver wants to save it for that device, it can.
>
> thanks,
>
> greg k-h


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-05-30 12:14     ` Takashi Sakamoto
@ 2013-06-02 22:27       ` Stefan Richter
  2013-06-02 22:32         ` [PATCH] firewire: remove support of fw_driver.driver.probe " Stefan Richter
                           ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Stefan Richter @ 2013-06-02 22:27 UTC (permalink / raw)
  To: Takashi Sakamoto, linux1394-devel
  Cc: Greg Kroah-Hartman, linux-kernel, Clemens Ladisch, Peter Hurley

FireWire upper layer drivers are converted from generic
    struct driver.probe() and .remove()
to bus-specific
    struct fw_driver.probe() and .remove().

The new .probe() adds a const struct ieee1394_device_id *id argument,
indicating the entry in the driver's device identifiers table which
matched the fw_unit to be probed.  This new argument is used by the
snd-firewire-speakers driver to look up device-specific parameters and
methods.  There is at least one other FireWire audio driver currently in
development in which this will be useful too.

The new .remove() drops the unused error return code.

Although all in-tree drivers are being converted to the new methods,
support for the old methods is left in place for the time being, aiming
to avoid conflicts if/when new drivers are being merged into the
mainline via other trees.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Clemens Ladisch <clemens@ladisch.de>
Cc: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/firewire/core-device.c      |   42 +++++++++--
 drivers/firewire/net.c              |   50 ++++++-------
 drivers/firewire/sbp2.c             |   17 ++--
 drivers/media/firewire/firedtv-fw.c |   19 ++---
 drivers/staging/fwserial/fwserial.c |   14 ++--
 include/linux/firewire.h            |    2 +
 sound/firewire/isight.c             |   44 ++++++------
 sound/firewire/scs1x.c              |   39 +++++-----
 sound/firewire/speakers.c           |  106 +++++++++++-----------------
 9 files changed, 166 insertions(+), 167 deletions(-)

--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -167,23 +167,51 @@ static bool match_ids(const struct ieee1
 
 static bool is_fw_unit(struct device *dev);
 
-static int fw_unit_match(struct device *dev, struct device_driver *drv)
+static const struct ieee1394_device_id *unit_match(struct device *dev,
+						   struct device_driver *drv)
 {
 	const struct ieee1394_device_id *id_table =
 			container_of(drv, struct fw_driver, driver)->id_table;
 	int id[] = {0, 0, 0, 0};
 
+	get_modalias_ids(fw_unit(dev), id);
+
+	for (; id_table->match_flags != 0; id_table++)
+		if (match_ids(id_table, id))
+			return id_table;
+
+	return NULL;
+}
+
+static int fw_unit_match(struct device *dev, struct device_driver *drv)
+{
 	/* We only allow binding to fw_units. */
 	if (!is_fw_unit(dev))
 		return 0;
 
-	get_modalias_ids(fw_unit(dev), id);
+	return !!unit_match(dev, drv);
+}
 
-	for (; id_table->match_flags != 0; id_table++)
-		if (match_ids(id_table, id))
-			return 1;
+static int fw_unit_probe(struct device *dev)
+{
+	struct fw_driver *driver =
+			container_of(dev->driver, struct fw_driver, driver);
 
-	return 0;
+	if (driver->probe)
+		return driver->probe(fw_unit(dev), unit_match(dev, dev->driver));
+	else
+		return driver->driver.probe(dev);
+}
+
+static int fw_unit_remove(struct device *dev)
+{
+	struct fw_driver *driver =
+			container_of(dev->driver, struct fw_driver, driver);
+
+	if (driver->remove)
+		return driver->remove(fw_unit(dev)), 0;
+	else
+		return driver->driver.remove(dev);
 }
 
 static int get_modalias(struct fw_unit *unit, char *buffer, size_t buffer_size)
@@ -213,6 +241,8 @@ static int fw_unit_uevent(struct device
 struct bus_type fw_bus_type = {
 	.name = "firewire",
 	.match = fw_unit_match,
+	.probe = fw_unit_probe,
+	.remove = fw_unit_remove,
 };
 EXPORT_SYMBOL(fw_bus_type);
 
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1440,9 +1440,9 @@ static int fwnet_add_peer(struct fwnet_d
 	return 0;
 }
 
-static int fwnet_probe(struct device *_dev)
+static int fwnet_probe(struct fw_unit *unit,
+		       const struct ieee1394_device_id *id)
 {
-	struct fw_unit *unit = fw_unit(_dev);
 	struct fw_device *device = fw_parent_device(unit);
 	struct fw_card *card = device->card;
 	struct net_device *net;
@@ -1526,6 +1526,24 @@ static int fwnet_probe(struct device *_d
 	return ret;
 }
 
+/*
+ * FIXME abort partially sent fragmented datagrams,
+ * discard partially received fragmented datagrams
+ */
+static void fwnet_update(struct fw_unit *unit)
+{
+	struct fw_device *device = fw_parent_device(unit);
+	struct fwnet_peer *peer = dev_get_drvdata(&unit->device);
+	int generation;
+
+	generation = device->generation;
+
+	spin_lock_irq(&peer->dev->lock);
+	peer->node_id    = device->node_id;
+	peer->generation = generation;
+	spin_unlock_irq(&peer->dev->lock);
+}
+
 static void fwnet_remove_peer(struct fwnet_peer *peer, struct fwnet_device *dev)
 {
 	struct fwnet_partial_datagram *pd, *pd_next;
@@ -1542,9 +1560,9 @@ static void fwnet_remove_peer(struct fwn
 	kfree(peer);
 }
 
-static int fwnet_remove(struct device *_dev)
+static void fwnet_remove(struct fw_unit *unit)
 {
-	struct fwnet_peer *peer = dev_get_drvdata(_dev);
+	struct fwnet_peer *peer = dev_get_drvdata(&unit->device);
 	struct fwnet_device *dev = peer->dev;
 	struct net_device *net;
 	int i;
@@ -1569,26 +1587,6 @@ static int fwnet_remove(struct device *_
 	}
 
 	mutex_unlock(&fwnet_device_mutex);
-
-	return 0;
-}
-
-/*
- * FIXME abort partially sent fragmented datagrams,
- * discard partially received fragmented datagrams
- */
-static void fwnet_update(struct fw_unit *unit)
-{
-	struct fw_device *device = fw_parent_device(unit);
-	struct fwnet_peer *peer = dev_get_drvdata(&unit->device);
-	int generation;
-
-	generation = device->generation;
-
-	spin_lock_irq(&peer->dev->lock);
-	peer->node_id    = device->node_id;
-	peer->generation = generation;
-	spin_unlock_irq(&peer->dev->lock);
 }
 
 static const struct ieee1394_device_id fwnet_id_table[] = {
@@ -1614,10 +1612,10 @@ static struct fw_driver fwnet_driver = {
 		.owner  = THIS_MODULE,
 		.name   = KBUILD_MODNAME,
 		.bus    = &fw_bus_type,
-		.probe  = fwnet_probe,
-		.remove = fwnet_remove,
 	},
+	.probe    = fwnet_probe,
 	.update   = fwnet_update,
+	.remove   = fwnet_remove,
 	.id_table = fwnet_id_table,
 };
 
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -1128,11 +1128,10 @@ static void sbp2_init_workarounds(struct
 }
 
 static struct scsi_host_template scsi_driver_template;
-static int sbp2_remove(struct device *dev);
+static void sbp2_remove(struct fw_unit *unit);
 
-static int sbp2_probe(struct device *dev)
+static int sbp2_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 {
-	struct fw_unit *unit = fw_unit(dev);
 	struct fw_device *device = fw_parent_device(unit);
 	struct sbp2_target *tgt;
 	struct sbp2_logical_unit *lu;
@@ -1196,7 +1195,7 @@ static int sbp2_probe(struct device *dev
 	return 0;
 
  fail_remove:
-	sbp2_remove(dev);
+	sbp2_remove(unit);
 	return -ENOMEM;
 
  fail_shost_put:
@@ -1222,9 +1221,8 @@ static void sbp2_update(struct fw_unit *
 	}
 }
 
-static int sbp2_remove(struct device *dev)
+static void sbp2_remove(struct fw_unit *unit)
 {
-	struct fw_unit *unit = fw_unit(dev);
 	struct fw_device *device = fw_parent_device(unit);
 	struct sbp2_target *tgt = dev_get_drvdata(&unit->device);
 	struct sbp2_logical_unit *lu, *next;
@@ -1261,10 +1259,9 @@ static int sbp2_remove(struct device *de
 		kfree(lu);
 	}
 	scsi_remove_host(shost);
-	dev_notice(dev, "released target %d:0:0\n", shost->host_no);
+	dev_notice(&unit->device, "released target %d:0:0\n", shost->host_no);
 
 	scsi_host_put(shost);
-	return 0;
 }
 
 #define SBP2_UNIT_SPEC_ID_ENTRY	0x0000609e
@@ -1285,10 +1282,10 @@ static struct fw_driver sbp2_driver = {
 		.owner  = THIS_MODULE,
 		.name   = KBUILD_MODNAME,
 		.bus    = &fw_bus_type,
-		.probe  = sbp2_probe,
-		.remove = sbp2_remove,
 	},
+	.probe    = sbp2_probe,
 	.update   = sbp2_update,
+	.remove   = sbp2_remove,
 	.id_table = sbp2_id_table,
 };
 
--- a/drivers/media/firewire/firedtv-fw.c
+++ b/drivers/media/firewire/firedtv-fw.c
@@ -248,7 +248,7 @@ static const char * const model_names[]
 /* Adjust the template string if models with longer names appear. */
 #define MAX_MODEL_NAME_LEN sizeof("FireDTV ????")
 
-static int node_probe(struct device *dev)
+static int node_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 {
 	struct firedtv *fdtv;
 	char name[MAX_MODEL_NAME_LEN];
@@ -258,8 +258,8 @@ static int node_probe(struct device *dev
 	if (!fdtv)
 		return -ENOMEM;
 
-	dev_set_drvdata(dev, fdtv);
-	fdtv->device		= dev;
+	dev_set_drvdata(&unit->device, fdtv);
+	fdtv->device		= &unit->device;
 	fdtv->isochannel	= -1;
 	fdtv->voltage		= 0xff;
 	fdtv->tone		= 0xff;
@@ -269,7 +269,7 @@ static int node_probe(struct device *dev
 	mutex_init(&fdtv->demux_mutex);
 	INIT_WORK(&fdtv->remote_ctrl_work, avc_remote_ctrl_work);
 
-	name_len = fw_csr_string(fw_unit(dev)->directory, CSR_MODEL,
+	name_len = fw_csr_string(unit->directory, CSR_MODEL,
 				 name, sizeof(name));
 	for (i = ARRAY_SIZE(model_names); --i; )
 		if (strlen(model_names[i]) <= name_len &&
@@ -277,7 +277,7 @@ static int node_probe(struct device *dev
 			break;
 	fdtv->type = i;
 
-	err = fdtv_register_rc(fdtv, dev);
+	err = fdtv_register_rc(fdtv, &unit->device);
 	if (err)
 		goto fail_free;
 
@@ -307,9 +307,9 @@ fail_free:
 	return err;
 }
 
-static int node_remove(struct device *dev)
+static void node_remove(struct fw_unit *unit)
 {
-	struct firedtv *fdtv = dev_get_drvdata(dev);
+	struct firedtv *fdtv = dev_get_drvdata(&unit->device);
 
 	fdtv_dvb_unregister(fdtv);
 
@@ -320,7 +320,6 @@ static int node_remove(struct device *de
 	fdtv_unregister_rc(fdtv);
 
 	kfree(fdtv);
-	return 0;
 }
 
 static void node_update(struct fw_unit *unit)
@@ -391,10 +390,10 @@ static struct fw_driver fdtv_driver = {
 		.owner  = THIS_MODULE,
 		.name   = "firedtv",
 		.bus    = &fw_bus_type,
-		.probe  = node_probe,
-		.remove = node_remove,
 	},
+	.probe    = node_probe,
 	.update   = node_update,
+	.remove   = node_remove,
 	.id_table = fdtv_id_table,
 };
 
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -2438,9 +2438,9 @@ free_ports:
  * last peer for a given fw_card triggering the destruction of the same
  * fw_serial for the same fw_card.
  */
-static int fwserial_probe(struct device *dev)
+static int fwserial_probe(struct fw_unit *unit,
+			  const struct ieee1394_device_id *id)
 {
-	struct fw_unit *unit = fw_unit(dev);
 	struct fw_serial *serial;
 	int err;
 
@@ -2462,9 +2462,9 @@ static int fwserial_probe(struct device
  * specific fw_card). If this is the last peer being removed, then trigger
  * the destruction of the underlying TTYs.
  */
-static int fwserial_remove(struct device *dev)
+static void fwserial_remove(struct fw_unit *unit)
 {
-	struct fwtty_peer *peer = dev_get_drvdata(dev);
+	struct fwtty_peer *peer = dev_get_drvdata(&unit->device);
 	struct fw_serial *serial = peer->serial;
 	int i;
 
@@ -2484,8 +2484,6 @@ static int fwserial_remove(struct device
 		kref_put(&serial->kref, fwserial_destroy);
 	}
 	mutex_unlock(&fwserial_list_mutex);
-
-	return 0;
 }
 
 /**
@@ -2530,10 +2528,10 @@ static struct fw_driver fwserial_driver
 		.owner  = THIS_MODULE,
 		.name   = KBUILD_MODNAME,
 		.bus    = &fw_bus_type,
-		.probe  = fwserial_probe,
-		.remove = fwserial_remove,
 	},
+	.probe    = fwserial_probe,
 	.update   = fwserial_update,
+	.remove   = fwserial_remove,
 	.id_table = fwserial_id_table,
 };
 
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -251,8 +251,10 @@ struct ieee1394_device_id;
 
 struct fw_driver {
 	struct device_driver driver;
+	int (*probe)(struct fw_unit *unit, const struct ieee1394_device_id *id);
 	/* Called when the parent device sits through a bus reset. */
 	void (*update)(struct fw_unit *unit);
+	void (*remove)(struct fw_unit *unit);
 	const struct ieee1394_device_id *id_table;
 };
 
--- a/sound/firewire/isight.c
+++ b/sound/firewire/isight.c
@@ -626,9 +626,9 @@ static u64 get_unit_base(struct fw_unit
 	return 0;
 }
 
-static int isight_probe(struct device *unit_dev)
+static int isight_probe(struct fw_unit *unit,
+			const struct ieee1394_device_id *id)
 {
-	struct fw_unit *unit = fw_unit(unit_dev);
 	struct fw_device *fw_dev = fw_parent_device(unit);
 	struct snd_card *card;
 	struct isight *isight;
@@ -637,7 +637,7 @@ static int isight_probe(struct device *u
 	err = snd_card_create(-1, NULL, THIS_MODULE, sizeof(*isight), &card);
 	if (err < 0)
 		return err;
-	snd_card_set_dev(card, unit_dev);
+	snd_card_set_dev(card, &unit->device);
 
 	isight = card->private_data;
 	isight->card = card;
@@ -674,7 +674,7 @@ static int isight_probe(struct device *u
 	if (err < 0)
 		goto error;
 
-	dev_set_drvdata(unit_dev, isight);
+	dev_set_drvdata(&unit->device, isight);
 
 	return 0;
 
@@ -686,23 +686,6 @@ error:
 	return err;
 }
 
-static int isight_remove(struct device *dev)
-{
-	struct isight *isight = dev_get_drvdata(dev);
-
-	isight_pcm_abort(isight);
-
-	snd_card_disconnect(isight->card);
-
-	mutex_lock(&isight->mutex);
-	isight_stop_streaming(isight);
-	mutex_unlock(&isight->mutex);
-
-	snd_card_free_when_closed(isight->card);
-
-	return 0;
-}
-
 static void isight_bus_reset(struct fw_unit *unit)
 {
 	struct isight *isight = dev_get_drvdata(&unit->device);
@@ -716,6 +699,21 @@ static void isight_bus_reset(struct fw_u
 	}
 }
 
+static void isight_remove(struct fw_unit *unit)
+{
+	struct isight *isight = dev_get_drvdata(&unit->device);
+
+	isight_pcm_abort(isight);
+
+	snd_card_disconnect(isight->card);
+
+	mutex_lock(&isight->mutex);
+	isight_stop_streaming(isight);
+	mutex_unlock(&isight->mutex);
+
+	snd_card_free_when_closed(isight->card);
+}
+
 static const struct ieee1394_device_id isight_id_table[] = {
 	{
 		.match_flags  = IEEE1394_MATCH_SPECIFIER_ID |
@@ -732,10 +730,10 @@ static struct fw_driver isight_driver =
 		.owner	= THIS_MODULE,
 		.name	= KBUILD_MODNAME,
 		.bus	= &fw_bus_type,
-		.probe	= isight_probe,
-		.remove	= isight_remove,
 	},
+	.probe    = isight_probe,
 	.update   = isight_bus_reset,
+	.remove   = isight_remove,
 	.id_table = isight_id_table,
 };
 
--- a/sound/firewire/scs1x.c
+++ b/sound/firewire/scs1x.c
@@ -384,9 +384,8 @@ static void scs_card_free(struct snd_car
 	kfree(scs->buffer);
 }
 
-static int scs_probe(struct device *unit_dev)
+static int scs_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 {
-	struct fw_unit *unit = fw_unit(unit_dev);
 	struct fw_device *fw_dev = fw_parent_device(unit);
 	struct snd_card *card;
 	struct scs *scs;
@@ -395,7 +394,7 @@ static int scs_probe(struct device *unit
 	err = snd_card_create(-16, NULL, THIS_MODULE, sizeof(*scs), &card);
 	if (err < 0)
 		return err;
-	snd_card_set_dev(card, unit_dev);
+	snd_card_set_dev(card, &unit->device);
 
 	scs = card->private_data;
 	scs->card = card;
@@ -440,7 +439,7 @@ static int scs_probe(struct device *unit
 	if (err < 0)
 		goto err_card;
 
-	dev_set_drvdata(unit_dev, scs);
+	dev_set_drvdata(&unit->device, scs);
 
 	return 0;
 
@@ -451,9 +450,20 @@ err_card:
 	return err;
 }
 
-static int scs_remove(struct device *dev)
+static void scs_update(struct fw_unit *unit)
 {
-	struct scs *scs = dev_get_drvdata(dev);
+	struct scs *scs = dev_get_drvdata(&unit->device);
+	__be64 data;
+
+	data = cpu_to_be64(((u64)HSS1394_TAG_CHANGE_ADDRESS << 56) |
+			   scs->hss_handler.offset);
+	snd_fw_transaction(scs->unit, TCODE_WRITE_BLOCK_REQUEST,
+			   HSS1394_ADDRESS, &data, 8);
+}
+
+static void scs_remove(struct fw_unit *unit)
+{
+	struct scs *scs = dev_get_drvdata(&unit->device);
 
 	snd_card_disconnect(scs->card);
 
@@ -465,19 +475,6 @@ static int scs_remove(struct device *dev
 	tasklet_kill(&scs->tasklet);
 
 	snd_card_free_when_closed(scs->card);
-
-	return 0;
-}
-
-static void scs_update(struct fw_unit *unit)
-{
-	struct scs *scs = dev_get_drvdata(&unit->device);
-	__be64 data;
-
-	data = cpu_to_be64(((u64)HSS1394_TAG_CHANGE_ADDRESS << 56) |
-			   scs->hss_handler.offset);
-	snd_fw_transaction(scs->unit, TCODE_WRITE_BLOCK_REQUEST,
-			   HSS1394_ADDRESS, &data, 8);
 }
 
 static const struct ieee1394_device_id scs_id_table[] = {
@@ -506,10 +503,10 @@ static struct fw_driver scs_driver = {
 		.owner  = THIS_MODULE,
 		.name   = KBUILD_MODNAME,
 		.bus    = &fw_bus_type,
-		.probe  = scs_probe,
-		.remove = scs_remove,
 	},
+	.probe    = scs_probe,
 	.update   = scs_update,
+	.remove   = scs_remove,
 	.id_table = scs_id_table,
 };
 
--- a/sound/firewire/speakers.c
+++ b/sound/firewire/speakers.c
@@ -663,45 +663,9 @@ static void fwspk_card_free(struct snd_c
 	mutex_destroy(&fwspk->mutex);
 }
 
-static const struct device_info *fwspk_detect(struct fw_device *dev)
+static int fwspk_probe(struct fw_unit *unit,
+		       const struct ieee1394_device_id *id)
 {
-	static const struct device_info griffin_firewave = {
-		.driver_name = "FireWave",
-		.short_name  = "FireWave",
-		.long_name   = "Griffin FireWave Surround",
-		.pcm_constraints = firewave_constraints,
-		.mixer_channels = 6,
-		.mute_fb_id   = 0x01,
-		.volume_fb_id = 0x02,
-	};
-	static const struct device_info lacie_speakers = {
-		.driver_name = "FWSpeakers",
-		.short_name  = "FireWire Speakers",
-		.long_name   = "LaCie FireWire Speakers",
-		.pcm_constraints = lacie_speakers_constraints,
-		.mixer_channels = 1,
-		.mute_fb_id   = 0x01,
-		.volume_fb_id = 0x01,
-	};
-	struct fw_csr_iterator i;
-	int key, value;
-
-	fw_csr_iterator_init(&i, dev->config_rom);
-	while (fw_csr_iterator_next(&i, &key, &value))
-		if (key == CSR_VENDOR)
-			switch (value) {
-			case VENDOR_GRIFFIN:
-				return &griffin_firewave;
-			case VENDOR_LACIE:
-				return &lacie_speakers;
-			}
-
-	return NULL;
-}
-
-static int fwspk_probe(struct device *unit_dev)
-{
-	struct fw_unit *unit = fw_unit(unit_dev);
 	struct fw_device *fw_dev = fw_parent_device(unit);
 	struct snd_card *card;
 	struct fwspk *fwspk;
@@ -711,17 +675,13 @@ static int fwspk_probe(struct device *un
 	err = snd_card_create(-1, NULL, THIS_MODULE, sizeof(*fwspk), &card);
 	if (err < 0)
 		return err;
-	snd_card_set_dev(card, unit_dev);
+	snd_card_set_dev(card, &unit->device);
 
 	fwspk = card->private_data;
 	fwspk->card = card;
 	mutex_init(&fwspk->mutex);
 	fwspk->unit = fw_unit_get(unit);
-	fwspk->device_info = fwspk_detect(fw_dev);
-	if (!fwspk->device_info) {
-		err = -ENODEV;
-		goto err_unit;
-	}
+	fwspk->device_info = (const struct device_info *)id->driver_data;
 
 	err = cmp_connection_init(&fwspk->connection, unit, 0);
 	if (err < 0)
@@ -756,7 +716,7 @@ static int fwspk_probe(struct device *un
 	if (err < 0)
 		goto error;
 
-	dev_set_drvdata(unit_dev, fwspk);
+	dev_set_drvdata(&unit->device, fwspk);
 
 	return 0;
 
@@ -770,22 +730,6 @@ error:
 	return err;
 }
 
-static int fwspk_remove(struct device *dev)
-{
-	struct fwspk *fwspk = dev_get_drvdata(dev);
-
-	amdtp_out_stream_pcm_abort(&fwspk->stream);
-	snd_card_disconnect(fwspk->card);
-
-	mutex_lock(&fwspk->mutex);
-	fwspk_stop_stream(fwspk);
-	mutex_unlock(&fwspk->mutex);
-
-	snd_card_free_when_closed(fwspk->card);
-
-	return 0;
-}
-
 static void fwspk_bus_reset(struct fw_unit *unit)
 {
 	struct fwspk *fwspk = dev_get_drvdata(&unit->device);
@@ -803,6 +747,40 @@ static void fwspk_bus_reset(struct fw_un
 	amdtp_out_stream_update(&fwspk->stream);
 }
 
+static void fwspk_remove(struct fw_unit *unit)
+{
+	struct fwspk *fwspk = dev_get_drvdata(&unit->device);
+
+	amdtp_out_stream_pcm_abort(&fwspk->stream);
+	snd_card_disconnect(fwspk->card);
+
+	mutex_lock(&fwspk->mutex);
+	fwspk_stop_stream(fwspk);
+	mutex_unlock(&fwspk->mutex);
+
+	snd_card_free_when_closed(fwspk->card);
+}
+
+static const struct device_info griffin_firewave = {
+	.driver_name = "FireWave",
+	.short_name  = "FireWave",
+	.long_name   = "Griffin FireWave Surround",
+	.pcm_constraints = firewave_constraints,
+	.mixer_channels = 6,
+	.mute_fb_id   = 0x01,
+	.volume_fb_id = 0x02,
+};
+
+static const struct device_info lacie_speakers = {
+	.driver_name = "FWSpeakers",
+	.short_name  = "FireWire Speakers",
+	.long_name   = "LaCie FireWire Speakers",
+	.pcm_constraints = lacie_speakers_constraints,
+	.mixer_channels = 1,
+	.mute_fb_id   = 0x01,
+	.volume_fb_id = 0x01,
+};
+
 static const struct ieee1394_device_id fwspk_id_table[] = {
 	{
 		.match_flags  = IEEE1394_MATCH_VENDOR_ID |
@@ -813,6 +791,7 @@ static const struct ieee1394_device_id f
 		.model_id     = 0x00f970,
 		.specifier_id = SPECIFIER_1394TA,
 		.version      = VERSION_AVC,
+		.driver_data  = (kernel_ulong_t)&griffin_firewave,
 	},
 	{
 		.match_flags  = IEEE1394_MATCH_VENDOR_ID |
@@ -823,6 +802,7 @@ static const struct ieee1394_device_id f
 		.model_id     = 0x00f970,
 		.specifier_id = SPECIFIER_1394TA,
 		.version      = VERSION_AVC,
+		.driver_data  = (kernel_ulong_t)&lacie_speakers,
 	},
 	{ }
 };
@@ -833,10 +813,10 @@ static struct fw_driver fwspk_driver = {
 		.owner	= THIS_MODULE,
 		.name	= KBUILD_MODNAME,
 		.bus	= &fw_bus_type,
-		.probe	= fwspk_probe,
-		.remove	= fwspk_remove,
 	},
+	.probe    = fwspk_probe,
 	.update   = fwspk_bus_reset,
+	.remove   = fwspk_remove,
 	.id_table = fwspk_id_table,
 };
 


-- 
Stefan Richter
-=====-===-= -==- ---==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] firewire: remove support of fw_driver.driver.probe and .remove methods
  2013-06-02 22:27       ` [PATCH] firewire: introduce fw_driver.probe and .remove methods Stefan Richter
@ 2013-06-02 22:32         ` Stefan Richter
  2013-06-02 22:38         ` [PATCH] firewire: introduce fw_driver.probe " Greg Kroah-Hartman
  2013-06-03  7:17         ` Clemens Ladisch
  2 siblings, 0 replies; 12+ messages in thread
From: Stefan Richter @ 2013-06-02 22:32 UTC (permalink / raw)
  To: linux1394-devel; +Cc: Takashi Sakamoto, Greg Kroah-Hartman, linux-kernel

After all drivers being converted to bus-specific .probe/.remove methods,
remove support of the obsolete generic methods.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
To be merged maybe one release cycle after patch "firewire: introduce
fw_driver.probe and .remove methods".

 drivers/firewire/core-device.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -197,10 +197,7 @@ static int fw_unit_probe(struct device *
 	struct fw_driver *driver =
 			container_of(dev->driver, struct fw_driver, driver);
 
-	if (driver->probe)
-		return driver->probe(fw_unit(dev), unit_match(dev, dev->driver));
-	else
-		return driver->driver.probe(dev);
+	return driver->probe(fw_unit(dev), unit_match(dev, dev->driver));
 }
 
 static int fw_unit_remove(struct device *dev)
@@ -208,10 +205,9 @@ static int fw_unit_remove(struct device
 	struct fw_driver *driver =
 			container_of(dev->driver, struct fw_driver, driver);
 
-	if (driver->remove)
-		return driver->remove(fw_unit(dev)), 0;
-	else
-		return driver->driver.remove(dev);
+	driver->remove(fw_unit(dev));
+
+	return 0;
 }
 
 static int get_modalias(struct fw_unit *unit, char *buffer, size_t buffer_size)


-- 
Stefan Richter
-=====-===-= -==- ---==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-06-02 22:27       ` [PATCH] firewire: introduce fw_driver.probe and .remove methods Stefan Richter
  2013-06-02 22:32         ` [PATCH] firewire: remove support of fw_driver.driver.probe " Stefan Richter
@ 2013-06-02 22:38         ` Greg Kroah-Hartman
  2013-06-02 23:35           ` Stefan Richter
  2013-06-03  7:17         ` Clemens Ladisch
  2 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2013-06-02 22:38 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Takashi Sakamoto, linux1394-devel, linux-kernel, Clemens Ladisch,
	Peter Hurley

On Mon, Jun 03, 2013 at 12:27:49AM +0200, Stefan Richter wrote:
> FireWire upper layer drivers are converted from generic
>     struct driver.probe() and .remove()
> to bus-specific
>     struct fw_driver.probe() and .remove().
> 
> The new .probe() adds a const struct ieee1394_device_id *id argument,
> indicating the entry in the driver's device identifiers table which
> matched the fw_unit to be probed.  This new argument is used by the
> snd-firewire-speakers driver to look up device-specific parameters and
> methods.  There is at least one other FireWire audio driver currently in
> development in which this will be useful too.
> 
> The new .remove() drops the unused error return code.
> 
> Although all in-tree drivers are being converted to the new methods,
> support for the old methods is left in place for the time being, aiming
> to avoid conflicts if/when new drivers are being merged into the
> mainline via other trees.

Are there other firewire drivers in other trees in linux-next right now?
If not, I'd just recommend converting everything over and not using the
old functions at all.  Especially as there really isn't that many
firewire drivers, and new ones are pretty rare these days, right?

Other than that, your patch looks good.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-06-02 22:38         ` [PATCH] firewire: introduce fw_driver.probe " Greg Kroah-Hartman
@ 2013-06-02 23:35           ` Stefan Richter
  2013-06-03  3:59             ` Takashi Sakamoto
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Richter @ 2013-06-02 23:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Takashi Sakamoto, linux1394-devel, linux-kernel, Clemens Ladisch,
	Peter Hurley

On Jun 02 Greg Kroah-Hartman wrote:
> On Mon, Jun 03, 2013 at 12:27:49AM +0200, Stefan Richter wrote:
> > Although all in-tree drivers are being converted to the new methods,
> > support for the old methods is left in place for the time being, aiming
> > to avoid conflicts if/when new drivers are being merged into the
> > mainline via other trees.
> 
> Are there other firewire drivers in other trees in linux-next right now?
> If not, I'd just recommend converting everything over and not using the
> old functions at all.  Especially as there really isn't that many
> firewire drivers, and new ones are pretty rare these days, right?

I think there is none in -next at the moment.  But Sakamoto-san posted an
RFC just two days ago:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-June/062614.html

This is one of two(?) drivers on which he is currently working on; the
other one is the one which is going to need the new id argument
in .probe().  And then there is another person working on yet another
sound/firewire/ driver, but upstreaming of that driver might take a
little more time for some more integration steps, from what I heard.

So, alternatively I could fold "firewire: remove support of
fw_driver.driver.probe and .remove methods" into this one and commit it to
a stable topic branch in linux1394.git (to be merged early after 3.10),
and the audio driver developers could pull this topic branch if they want
to and when they want to.

> Other than that, your patch looks good.

Thanks for review.
-- 
Stefan Richter
-=====-===-= -==- ---==
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-06-02 23:35           ` Stefan Richter
@ 2013-06-03  3:59             ` Takashi Sakamoto
  2013-06-09 17:03               ` Stefan Richter
  0 siblings, 1 reply; 12+ messages in thread
From: Takashi Sakamoto @ 2013-06-03  3:59 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Greg Kroah-Hartman, linux1394-devel, linux-kernel,
	Clemens Ladisch, Peter Hurley

Hi Stefan and Greg,

I test Stefan's patches and it works well with my devices.

 > This is one of two(?) drivers on which he is currently working on;

Yes. I'm developing for two driver modules for some firewire sound 
device. One is "snd-fireworks" as Stefan mentioned and another is 
"snd-bebob" for BridgeCo.'s BeBoB based firewire sound devices. Both of 
them is now out of tree and in my github repository.

 >> Are there other firewire drivers in other trees in linux-next
 >> right now?

I know the persons who work for "snd-dice", an driver module is for TC 
Applied Technologies' Dice based firewire sound devices. I'll inform 
them about your patches and topic branch.


Thanks a lot!

Takashi Sakamoto
o-takashi@sakamocchi.jp

(Jun 03 2013 08:35), Stefan Richter wrote:
> On Jun 02 Greg Kroah-Hartman wrote:
>> On Mon, Jun 03, 2013 at 12:27:49AM +0200, Stefan Richter wrote:
>>> Although all in-tree drivers are being converted to the new methods,
>>> support for the old methods is left in place for the time being, aiming
>>> to avoid conflicts if/when new drivers are being merged into the
>>> mainline via other trees.
>>
>> Are there other firewire drivers in other trees in linux-next right now?
>> If not, I'd just recommend converting everything over and not using the
>> old functions at all.  Especially as there really isn't that many
>> firewire drivers, and new ones are pretty rare these days, right?
>
> I think there is none in -next at the moment.  But Sakamoto-san posted an
> RFC just two days ago:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-June/062614.html
>
> This is one of two(?) drivers on which he is currently working on; the
> other one is the one which is going to need the new id argument
> in .probe().  And then there is another person working on yet another
> sound/firewire/ driver, but upstreaming of that driver might take a
> little more time for some more integration steps, from what I heard.
>
> So, alternatively I could fold "firewire: remove support of
> fw_driver.driver.probe and .remove methods" into this one and commit it to
> a stable topic branch in linux1394.git (to be merged early after 3.10),
> and the audio driver developers could pull this topic branch if they want
> to and when they want to.
>
>> Other than that, your patch looks good.
>
> Thanks for review.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-06-02 22:27       ` [PATCH] firewire: introduce fw_driver.probe and .remove methods Stefan Richter
  2013-06-02 22:32         ` [PATCH] firewire: remove support of fw_driver.driver.probe " Stefan Richter
  2013-06-02 22:38         ` [PATCH] firewire: introduce fw_driver.probe " Greg Kroah-Hartman
@ 2013-06-03  7:17         ` Clemens Ladisch
  2 siblings, 0 replies; 12+ messages in thread
From: Clemens Ladisch @ 2013-06-03  7:17 UTC (permalink / raw)
  To: Stefan Richter
  Cc: Takashi Sakamoto, linux1394-devel, Greg Kroah-Hartman,
	linux-kernel, Peter Hurley

Stefan Richter wrote:
> FireWire upper layer drivers are converted from generic
>     struct driver.probe() and .remove()
> to bus-specific
>     struct fw_driver.probe() and .remove().

Acked-by: Clemens Ladisch <clemens@ladisch.de>
for these:
>  sound/firewire/isight.c             |   44 ++++++------
>  sound/firewire/scs1x.c              |   39 +++++-----
>  sound/firewire/speakers.c           |  106 +++++++++++-----------------

> Although all in-tree drivers are being converted to the new methods,
> support for the old methods is left in place for the time being, aiming
> to avoid conflicts if/when new drivers are being merged into the
> mainline via other trees.

AFAICS it's unlikely that any of the new sound drivers will be merged soon
enough for this to matter.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] firewire: introduce fw_driver.probe and .remove methods
  2013-06-03  3:59             ` Takashi Sakamoto
@ 2013-06-09 17:03               ` Stefan Richter
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Richter @ 2013-06-09 17:03 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Greg Kroah-Hartman, linux1394-devel, linux-kernel,
	Clemens Ladisch, Peter Hurley

I pushed the branch "fw_driver-api-transition" out to linux1394.git at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git fw_driver-api-transition

This branch originates from v3.10-rc5 and contains only the single patch
"firewire: introduce fw_driver.probe and .remove methods".

If you pull this branch, you _can_ move your drivers to the new methods.

Furthermore, the "master" and "for-next" branches at linux1394.git are
now based on "fw_driver-api-transition" and contain patch "firewire:
remove support of fw_driver.driver.probe and .remove methods".

This means that starting from tomorrow, any FireWire driver which goes into
linux-next _must_ implement the new methods.
-- 
Stefan Richter
-=====-===-= -==- -=--=
http://arcgraph.de/sr/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-06-09 17:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <51A20AEE.7060201@sakamocchi.jp>
2013-05-26 21:35 ` How to get driver_data of struct ieee1394_device_id in kernel driver module? Stefan Richter
2013-05-26 22:15   ` Stefan Richter
2013-05-26 22:57   ` Greg Kroah-Hartman
2013-05-29 18:09     ` Stefan Richter
2013-05-30 12:14     ` Takashi Sakamoto
2013-06-02 22:27       ` [PATCH] firewire: introduce fw_driver.probe and .remove methods Stefan Richter
2013-06-02 22:32         ` [PATCH] firewire: remove support of fw_driver.driver.probe " Stefan Richter
2013-06-02 22:38         ` [PATCH] firewire: introduce fw_driver.probe " Greg Kroah-Hartman
2013-06-02 23:35           ` Stefan Richter
2013-06-03  3:59             ` Takashi Sakamoto
2013-06-09 17:03               ` Stefan Richter
2013-06-03  7:17         ` Clemens Ladisch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.