Re: [PATCH v4] peak_pci: add support for PEAK-System PCIe/PCIeC/miniPCI cards

[Wolfgang Grandegger <wg@grandegger.com>]

On 02/01/2012 04:57 PM, Marc Kleine-Budde wrote:
> On 02/01/2012 04:55 PM, Stephane Grosjean wrote:
>> Le 01/02/2012 16:48, Marc Kleine-Budde a écrit :
>>> #if defined(CONFIG_I2C_ALGOBIT) || defined(CONFIG_I2C_ALGOBIT_MODULE)
>>> #define peak_pci_support_pcie()    (1)
>>> #else
>>> #define peak_pci_support_pcie()    (0)
>>> #endif
>>>
>>> In the probe function you can write:
>>>
>>> probe() {
>>>     ...
>>>
>>>     if (peak_pci_support_pcie()&&
>>>     pdev->device == PEAK_PCIEC_DEVICE_ID) {
>>>         ...
>>>     }
>>> }
>>>
>>> There should be no ifdefs left in the code. One question remains, is the
>>> i2c core clever enough to define no-ops if the i2c subsystem is switched
>>> off?
>>> I just tried - :( - it doesn't work
>>>
>>> drivers/net/can/sja1000/peak_pci.c: In function
>>> 'peak_pciec_write_pca9553':
>>> drivers/net/can/sja1000/peak_pci.c:267: error: implicit declaration of
>>> function 'i2c_transfer'
>>> drivers/net/can/sja1000/peak_pci.c: In function 'peak_pciec_init':
>>> drivers/net/can/sja1000/peak_pci.c:491: error: implicit declaration of
>>> function 'i2c_del_adapter'
>>>
>>> Marc
>>
>> What about that:
>>
>>> #if defined(CONFIG_I2C_ALGOBIT) || defined(CONFIG_I2C_ALGOBIT_MODULE)
>>> #define peak_pci_support_pcie()    (1)
>>> #else
>>> #define peak_pci_support_pcie()    (0)
>> #define i2_transfer(a, b, c)
>> #define i2c_del_adapter(a)

Why that? I think there is some confusion.

>>> #endif
> 
> This is an option, please make it static inline function, to have
> typechecking.
> 
> Wolfgang, what do you think?

Yes, static inline functions please. I think we should have:

#if defined(CONFIG_I2C_ALGOBIT) || defined(CONFIG_I2C_ALGOBIT_MODULE)
	/* or a real option selecting the LED support */

	... one ifdef block with the real implementation ...
#else
static inline int peak_pciec_led_init(...) {return -EINVAL;}
static inline void peak_pciec_led_init(...) {}
#endif

In the rest of the code, only the above two functions should be used.
Should work, if I have not overlooked something.

Hope it's clear what I mean.

Wolfgang.