Re: [PATH][RFC] acxsm: Make acx mdoular again

[Denis Vlasenko <vda@ilport.com.ua>]

[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]

On Sunday 26 February 2006 18:10, Carlos Martín wrote:
> Hi,
> 
> Well, here it is. I've not been able to run-test it yet, but it at least 
> compiles and loads without problems. This is on x86_64 compiled against 
> Linville's wireless-2.6 git tree. (I don't have any x86 boxes running Linux 
> right now, but it should be alright).
> 
> I've added a struct acx_ops with function pointers and deleted the functions 
> that just call the PCI/USB version depending on what we're using at the 
> moment. A few functions I've just made empty to simplify and not having to 
> check every time wether we do have it or not. These are only called on 
> load/unload and error conditions. Normal operation shouldn't suffer any speed 
> decrease it may be faster at times because we dereference a few pointers 
> instead of calling a couple of functions (just guessing, though).
> 
> The patch is 38K uncompressed. I've compressed and attached it and uploaded to 
> http://www.cmartin.tk/acx/acxsm-modularise.patch
> 
>    text    data     bss     dec     hex filename
>   55491     588       4   56083    db13 acx-common.ko
>   33523    1040       4   34567    8707 acx-pci.ko
>   17109    1008       0   18117    46c5 acx-usb.ko
> 
> Comments are welcome and I'll split the patch if needed.

usb.c
=====
#define BOGUS_SAFETY_PADDING 0x40

int
acxusb_s_issue_cmd_timeo(
        acx_device_t *adev,
        unsigned cmd,
        void *buffer,
        unsigned buflen,
        unsigned timeout)
{
#if ACX_DEBUG
        return acx_s_issue_cmd_timeo_debug(adev, cmd, buffer, buflen, timeout, __stringify(cmd));
#else
        return acx_s_issue_cmd_timeo_debug(adev, cmd, buffer, buflen, timeout);
#endif
}

stringify what? We want symbolic command name, not it's numeric value.
stringifying must occur in .h file.


acx_func.h
==========
#if ACX_DEBUG

/* We want to log cmd names */
int acxpci_s_issue_cmd_timeo_debug(acx_device_t *adev, unsigned cmd, void *param, unsigned len, unsigned timeout, const char* cmdstr);
int acxusb_s_issue_cmd_timeo_debug(acx_device_t *adev, unsigned cmd, void *param, unsigned len, unsigned timeout, const char* cmdstr);
static inline int
acx_s_issue_cmd_timeo_debug(acx_device_t *adev, unsigned cmd, void *param, unsigned len, unsigned timeout, const char* cmdstr)
{
        if (IS_PCI(adev))
                return acxpci_s_issue_cmd_timeo_debug(adev, cmd, param, len, timeout, cmdstr);
        return acxusb_s_issue_cmd_timeo_debug(adev, cmd, param, len, timeout, cmdstr);
}
int acx_s_issue_cmd(acx_device_t *adev, unsigned cmd, void *param, unsigned len);

It will recurse: acx_s_issue_cmd_timeo_debug -> acxusb_s_issue_cmd_timeo_debug -> acx_s_issue_cmd_timeo_debug ->....
Why do you need if (IS_PCI(adev)) at all?

Please apply the attached style fixes on top of your patches.

Patch is not applied. Sorry.
--
vda

[-- Attachment #2: acxsm-modularise2.patch --]
[-- Type: text/x-diff, Size: 2161 bytes --]

diff -urpN current_sm.1/acx_func.h current_sm.2/acx_func.h
--- current_sm.1/acx_func.h	Mon Feb 27 12:13:27 2006
+++ current_sm.2/acx_func.h	Mon Feb 27 12:10:32 2006
@@ -444,7 +444,7 @@ int acx_s_interrogate_debug(acx_device_t
 #else
 
 
-/* We can't make this a #define or inline because we need to referece it. */
+/* We can't make this a #define or inline because we need to reference it. */
 int acx_s_issue_cmd(acx_device_t *adev, unsigned cmd, void *param, unsigned len);
 
 int acxpci_s_issue_cmd_timeo(acx_device_t *adev, unsigned cmd, void *param, unsigned len, unsigned timeout);
diff -urpN current_sm.1/common.c current_sm.2/common.c
--- current_sm.1/common.c	Mon Feb 27 12:13:28 2006
+++ current_sm.2/common.c	Mon Feb 27 12:06:33 2006
@@ -331,10 +331,11 @@ acx_log_fn_exit_v(const char *funcname, 
 EXPORT_SYMBOL_GPL(acx_log_fn_exit_v);
 #endif /* ACX_DEBUG > 1 */
 
-int acx_s_issue_cmd(acx_device_t *adev, unsigned cmd, void *param, unsigned len)
+int
+acx_s_issue_cmd(acx_device_t *adev, unsigned cmd, void *param, unsigned len)
 {
-  return adev->ops.issue_cmd_timeo(adev, cmd, param, len,
-				   ACX_CMD_TIMEOUT_DEFAULT);
+	return adev->ops.issue_cmd_timeo(adev, cmd, param, len,
+				ACX_CMD_TIMEOUT_DEFAULT);
 }
 
 EXPORT_SYMBOL_GPL(acx_s_issue_cmd);
@@ -1940,7 +1941,7 @@ acx_s_scan_chan(acx_device_t *adev)
 	FN_EXIT0;
 }
 
- EXPORT_SYMBOL_GPL(acx_s_scan_chan);
+EXPORT_SYMBOL_GPL(acx_s_scan_chan);
 
 void
 acx_s_cmd_start_scan(acx_device_t *adev)
@@ -2059,7 +2060,7 @@ acx111_s_feature_set(acx_device_t *adev,
 /***********************************************************************
 ** acx100_s_init_memory_pools
 */
-int
+static int
 acx100_s_init_memory_pools(acx_device_t *adev, const acx_ie_memmap_t *mmt)
 {
 	acx100_ie_memblocksize_t MemoryBlockSize;
diff -urpN current_sm.1/pci.c current_sm.2/pci.c
--- current_sm.1/pci.c	Mon Feb 27 12:13:28 2006
+++ current_sm.2/pci.c	Mon Feb 27 12:08:30 2006
@@ -3089,7 +3089,11 @@ end:
 	return (tx_t*)txdesc;
 }
 
-void acxpci_l_dealloc_tx(tx_t *tx_opaque){}
+void
+acxpci_l_dealloc_tx(tx_t *tx_opaque)
+{
+}
+
 
 /***********************************************************************
 */