[PATCH 0/3] firewire: minor code refactoring for link internal data

[PATCH 0/3] firewire: minor code refactoring for link internal data

[Takashi Sakamoto]

Hi,

The current implementation of both core functions and 1394 OHCI driver has
some points that need to be refactored regarding the handling of link-
internal data.

This patchset is for the purpose.

Takashi Sakamoto (3):
  firewire: ohci: use TCODE_LINK_INTERNAL consistently
  firewire: ohci: minor code refactoring to localize text table
  firewire: core: use common helper function to serialize phy
    configuration packet

 drivers/firewire/core-cdev.c        |  4 +++-
 drivers/firewire/core-transaction.c |  2 +-
 drivers/firewire/ohci.c             | 37 +++++++++++++++++------------
 drivers/firewire/ohci.h             |  1 -
 4 files changed, 26 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH 1/3] firewire: ohci: use TCODE_LINK_INTERNAL consistently

[Takashi Sakamoto]

In IEEE 1394 specification, 0x0e in tcode field is reserved for internal
purpose depending on link layer. In 1394 OHCI specification, it is used to
express phy packet in AT/AR contexts.

Current implementation of 1394 OHCI driver has several macros for the code.
They can be simply replaced with a macro in core code.

This commit obsoletes the macros.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 9 ++++-----
 drivers/firewire/ohci.h | 1 -
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 314a29c0fd3e..c3fff94b13e5 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -264,7 +264,6 @@ static inline struct fw_ohci *fw_ohci(struct fw_card *card)
 #define OHCI1394_REGISTER_SIZE		0x800
 #define OHCI1394_PCI_HCI_Control	0x40
 #define SELF_ID_BUF_SIZE		0x800
-#define OHCI_TCODE_PHY_PACKET		0x0e
 #define OHCI_VERSION_1_1		0x010010
 
 static char ohci_driver_name[] = KBUILD_MODNAME;
@@ -586,7 +585,7 @@ static void log_ar_at_event(struct fw_ohci *ohci,
 		ohci_notice(ohci, "A%c %s, %s\n",
 			    dir, evts[evt], tcodes[tcode]);
 		break;
-	case 0xe:
+	case TCODE_LINK_INTERNAL:
 		ohci_notice(ohci, "A%c %s, PHY %08x %08x\n",
 			    dir, evts[evt], header[1], header[2]);
 		break;
@@ -939,7 +938,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
 
 	case TCODE_WRITE_RESPONSE:
 	case TCODE_READ_QUADLET_REQUEST:
-	case OHCI_TCODE_PHY_PACKET:
+	case TCODE_LINK_INTERNAL:
 		p.header_length = 12;
 		p.payload_length = 0;
 		break;
@@ -967,7 +966,7 @@ static __le32 *handle_ar_packet(struct ar_context *ctx, __le32 *buffer)
 	 * Several controllers, notably from NEC and VIA, forget to
 	 * write ack_complete status at PHY packet reception.
 	 */
-	if (evt == OHCI1394_evt_no_status && tcode == OHCI1394_phy_tcode)
+	if (evt == OHCI1394_evt_no_status && tcode == TCODE_LINK_INTERNAL)
 		p.ack = ACK_COMPLETE;
 
 	/*
@@ -1435,7 +1434,7 @@ static int at_context_queue_packet(struct context *ctx,
 		break;
 
 	case TCODE_LINK_INTERNAL:
-		header[0] = cpu_to_le32((OHCI1394_phy_tcode << 4) |
+		header[0] = cpu_to_le32((TCODE_LINK_INTERNAL << 4) |
 					(packet->speed << 16));
 		header[1] = cpu_to_le32(packet->header[1]);
 		header[2] = cpu_to_le32(packet->header[2]);
diff --git a/drivers/firewire/ohci.h b/drivers/firewire/ohci.h
index 71c2ed84cafb..9ed36cfc6cae 100644
--- a/drivers/firewire/ohci.h
+++ b/drivers/firewire/ohci.h
@@ -153,7 +153,6 @@
 #define OHCI1394_evt_unknown		0xe
 #define OHCI1394_evt_flushed		0xf
 
-#define OHCI1394_phy_tcode		0xe
 
 // Self-ID DMA.
 
-- 
2.43.0

[PATCH 2/3] firewire: ohci: minor code refactoring to localize text table

[Takashi Sakamoto]

The string table for tcode is just used by log_ar_at_event(). In the case,
it is suitable to move the table inner the function definition.

This commit is for the purpose. Additionally, the hard-coded value for
tcode is replaced with defined macros as many as possible.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/ohci.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index c3fff94b13e5..a0bb0e87e18a 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -531,20 +531,28 @@ static const char *evts[] = {
 	[0x1e] = "ack_type_error",	[0x1f] = "-reserved-",
 	[0x20] = "pending/cancelled",
 };
-static const char *tcodes[] = {
-	[0x0] = "QW req",		[0x1] = "BW req",
-	[0x2] = "W resp",		[0x3] = "-reserved-",
-	[0x4] = "QR req",		[0x5] = "BR req",
-	[0x6] = "QR resp",		[0x7] = "BR resp",
-	[0x8] = "cycle start",		[0x9] = "Lk req",
-	[0xa] = "async stream packet",	[0xb] = "Lk resp",
-	[0xc] = "-reserved-",		[0xd] = "-reserved-",
-	[0xe] = "link internal",	[0xf] = "-reserved-",
-};
 
 static void log_ar_at_event(struct fw_ohci *ohci,
 			    char dir, int speed, u32 *header, int evt)
 {
+	static const char *const tcodes[] = {
+		[TCODE_WRITE_QUADLET_REQUEST]	= "QW req",
+		[TCODE_WRITE_BLOCK_REQUEST]	= "BW req",
+		[TCODE_WRITE_RESPONSE]		= "W resp",
+		[0x3]				= "-reserved-",
+		[TCODE_READ_QUADLET_REQUEST]	= "QR req",
+		[TCODE_READ_BLOCK_REQUEST]	= "BR req",
+		[TCODE_READ_QUADLET_RESPONSE]	= "QR resp",
+		[TCODE_READ_BLOCK_RESPONSE]	= "BR resp",
+		[TCODE_CYCLE_START]		= "cycle start",
+		[TCODE_LOCK_REQUEST]		= "Lk req",
+		[TCODE_STREAM_DATA]		= "async stream packet",
+		[TCODE_LOCK_RESPONSE]		= "Lk resp",
+		[0xc]				= "-reserved-",
+		[0xd]				= "-reserved-",
+		[TCODE_LINK_INTERNAL]		= "link internal",
+		[0xf]				= "-reserved-",
+	};
 	int tcode = async_header_get_tcode(header);
 	char specific[12];
 
-- 
2.43.0

[PATCH 3/3] firewire: core: use common helper function to serialize phy configuration packet

[Takashi Sakamoto]

A common helper function is available to serialize the first quadlet of phy
configuration packet.

This commit is for the purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-cdev.c        | 4 +++-
 drivers/firewire/core-transaction.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 9a7dc90330a3..619048dcfd72 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -37,6 +37,8 @@
 #include "core.h"
 #include <trace/events/firewire.h>
 
+#include "packet-header-definitions.h"
+
 /*
  * ABI version history is documented in linux/firewire-cdev.h.
  */
@@ -1635,7 +1637,7 @@ static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg)
 	e->client		= client;
 	e->p.speed		= SCODE_100;
 	e->p.generation		= a->generation;
-	e->p.header[0]		= TCODE_LINK_INTERNAL << 4;
+	async_header_set_tcode(e->p.header, TCODE_LINK_INTERNAL);
 	e->p.header[1]		= a->data[0];
 	e->p.header[2]		= a->data[1];
 	e->p.header_length	= 12;
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 4d2fc1f31fec..a89c841a7dbe 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -464,7 +464,6 @@ static void transmit_phy_packet_callback(struct fw_packet *packet,
 
 static struct fw_packet phy_config_packet = {
 	.header_length	= 12,
-	.header[0]	= TCODE_LINK_INTERNAL << 4,
 	.payload_length	= 0,
 	.speed		= SCODE_100,
 	.callback	= transmit_phy_packet_callback,
@@ -497,6 +496,7 @@ void fw_send_phy_config(struct fw_card *card,
 
 	mutex_lock(&phy_config_mutex);
 
+	async_header_set_tcode(phy_config_packet.header, TCODE_LINK_INTERNAL);
 	phy_config_packet.header[1] = data;
 	phy_config_packet.header[2] = ~data;
 	phy_config_packet.generation = generation;
-- 
2.43.0

Re: [PATCH 0/3] firewire: minor code refactoring for link internal data

[Takashi Sakamoto]

On Mon, Jul 29, 2024 at 10:46:28PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> The current implementation of both core functions and 1394 OHCI driver has
> some points that need to be refactored regarding the handling of link-
> internal data.
> 
> This patchset is for the purpose.
> 
> Takashi Sakamoto (3):
>   firewire: ohci: use TCODE_LINK_INTERNAL consistently
>   firewire: ohci: minor code refactoring to localize text table
>   firewire: core: use common helper function to serialize phy
>     configuration packet
> 
>  drivers/firewire/core-cdev.c        |  4 +++-
>  drivers/firewire/core-transaction.c |  2 +-
>  drivers/firewire/ohci.c             | 37 +++++++++++++++++------------
>  drivers/firewire/ohci.h             |  1 -
>  4 files changed, 26 insertions(+), 18 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto