Date:   Thu, 4 Nov 2021 14:19:26 +0300
From:   Denis Kirjanov <dkirjanov@...e.de>
To:     Michael Schmitz <schmitzmic@...il.com>, linux-m68k@...r.kernel.org,
        geert@...ux-m68k.org
Cc:     alex@...ik.de, netdev@...r.kernel.org
Subject: Re: [PATCH v8 3/3] net/8390: apne.c - add 100 Mbit support to apne.c
 driver



11/4/21 9:10 AM, Michael Schmitz пишет:
> Add module parameter, IO mode autoprobe and PCMCIA reset code
> required to support 100 Mbit PCMCIA ethernet cards on Amiga.
> 
> Select core PCMCIA support modules for use by APNE driver.
> 
> 10 Mbit and 100 Mbit mode are supported by the same module.
> Use the core PCMCIA cftable parser to detect 16 bit cards,
> and automatically enable 16 bit ISA IO access for those cards
> by changing isa_type at runtime. Code to reset the PCMCIA
> hardware required for 16 bit cards is also added to the driver
> probe.
> 
> An optional module parameter switches Amiga ISA IO accessors
> to 8 or 16 bit access in case autoprobe fails.
> 
> Patch modified after patch "[PATCH RFC net-next] Amiga PCMCIA
> 100 MBit card support" submitted to netdev 2018/09/16 by Alex
> Kazik <alex@...ik.de>.
> 
> CC: netdev@...r.kernel.org
> Link: https://lore.kernel.org/r/1622958877-2026-1-git-send-email-schmitzmic@gmail.com
> Tested-by: Alex Kazik <alex@...ik.de>
> Signed-off-by: Michael Schmitz <schmitzmic@...il.com>
> 
> --
> 
> Changes from v7:
> 
> - move 'select' for PCCARD and PCMCIA to 8390 Kconfig, so
>    Amiga pcmcia.c may remain built-in while core PCMCIA
>    code can be built as modules if APNE driver is a module.
> - move 16 bit mode autoprobe code from amiga/pcmcia.c to this
>    driver, to allow the core PCMCIA code we depend on to be
>    built as modules.
> - change module parameter type from bool to int to allow for
>    tri-state semantics (autoprobe, 8 bit, 16 bit).
> 
> Changes from v6:
> 
> - use 16 bit mode autoprobe based on PCMCIA config table data
> 
> Changes from v5:
> 
> - move autoprobe code to new patch in this series
> 
> Geert Uytterhoeven:
> - reword Kconfig help text
> 
> Finn Thain:
> - style fixes, use msec_to_jiffies in timeout calc
> 
> Alex Kazik:
> - revert module parameter permission change
> 
> Changes from v4:
> 
> Geert Uytterhoeven:
> - remove APNE100MBIT config option, always include 16 bit support
> - change module parameter permissions
> - try autoprobing for 16 bit mode early on in device probe
> 
> Changes from v3:
> 
> - change module parameter name to match Kconfig help
> 
> Finn Thain:
> - fix coding style in new card reset code block
> - allow reset of isa_type by module parameter
> 
> Changes from v1:
> 
> - fix module parameter name in Kconfig help text
> 
> Alex Kazik:
> - change module parameter type to bool, fix module parameter
>    permission
> 
> Changes from RFC:
> 
> Geert Uytterhoeven:
> - change APNE_100MBIT to depend on APNE
> - change '---help---' to 'help' (former no longer supported)
> - fix whitespace errors
> - fix module_param_named() arg count
> - protect all added code by #ifdef CONFIG_APNE_100MBIT
> 
> change module parameter - 0 for 8 bit, 1 for 16 bit, else autoprobe
> ---
>   drivers/net/ethernet/8390/Kconfig |  9 ++++
>   drivers/net/ethernet/8390/apne.c  | 69 +++++++++++++++++++++++++++++++
>   2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/net/ethernet/8390/Kconfig b/drivers/net/ethernet/8390/Kconfig
> index a4130e643342..b22c3cf96560 100644
> --- a/drivers/net/ethernet/8390/Kconfig
> +++ b/drivers/net/ethernet/8390/Kconfig
> @@ -136,6 +136,8 @@ config NE2K_PCI
>   config APNE
>   	tristate "PCMCIA NE2000 support"
>   	depends on AMIGA_PCMCIA
> +	select PCCARD
> +	select PCMCIA
>   	select CRC32
>   	help
>   	  If you have a PCMCIA NE2000 compatible adapter, say Y.  Otherwise,
> @@ -144,6 +146,13 @@ config APNE
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called apne.
>   
> +	  The driver also supports 10/100Mbit cards (e.g. Netgear FA411,
> +	  CNet Singlepoint). To activate 100 Mbit support, use the kernel
> +	  option apne.100mbit=1 (builtin) at boot time, or the apne.100mbit
> +	  module parameter. The driver will attempt to autoprobe 100 Mbit
> +	  mode, so this option may not be necessary. Use apne.100mbit=0
> +	  should autoprobe mis-detect a 100 Mbit card.
> +
>   config PCMCIA_PCNET
>   	tristate "NE2000 compatible PCMCIA support"
>   	depends on PCMCIA
> diff --git a/drivers/net/ethernet/8390/apne.c b/drivers/net/ethernet/8390/apne.c
> index da1ae37a9d73..115c008a129c 100644
> --- a/drivers/net/ethernet/8390/apne.c
> +++ b/drivers/net/ethernet/8390/apne.c
> @@ -38,6 +38,7 @@
>   #include <linux/etherdevice.h>
>   #include <linux/interrupt.h>
>   #include <linux/jiffies.h>
> +#include <pcmcia/cistpl.h>
>   
>   #include <asm/io.h>
>   #include <asm/setup.h>
> @@ -87,6 +88,7 @@ static void apne_block_output(struct net_device *dev, const int count,
>   static irqreturn_t apne_interrupt(int irq, void *dev_id);
>   
>   static int init_pcmcia(void);
> +static int pcmcia_is_16bit(void);
>   
>   /* IO base address used for nic */
>   
> @@ -119,6 +121,10 @@ static u32 apne_msg_enable;
>   module_param_named(msg_enable, apne_msg_enable, uint, 0444);
>   MODULE_PARM_DESC(msg_enable, "Debug message level (see linux/netdevice.h for bitmap)");
>   
> +static u32 apne_100_mbit = -1;
> +module_param_named(100_mbit, apne_100_mbit, uint, 0444);
> +MODULE_PARM_DESC(100_mbit, "Enable 100 Mbit support");

Can we try to detect 100mbit support by default on load and make a 
configuration option in sysfs?

> +
>   static struct net_device * __init apne_probe(void)
>   {
>   	struct net_device *dev;
> @@ -140,6 +146,13 @@ static struct net_device * __init apne_probe(void)
>   
>   	pr_info("Looking for PCMCIA ethernet card : ");
>   
> +	if (apne_100_mbit == 1)
> +		isa_type = ISA_TYPE_AG16;
> +	else if (apne_100_mbit == 0)
> +		isa_type = ISA_TYPE_AG;
> +	else
> +		pr_cont(" (autoprobing 16 bit mode) ");
> +
>   	/* check if a card is inserted */
>   	if (!(PCMCIA_INSERTED)) {
>   		pr_cont("NO PCMCIA card inserted\n");
> @@ -167,6 +180,14 @@ static struct net_device * __init apne_probe(void)
>   
>   	pr_cont("ethernet PCMCIA card inserted\n");
>   
> +#if IS_ENABLED(CONFIG_PCMCIA)
> +	if (apne_100_mbit < 0 && pcmcia_is_16bit()) {
> +		pr_info("16-bit PCMCIA card detected!\n");
> +		isa_type = ISA_TYPE_AG16;
> +		apne_100_mbit = 1;
> +	}
> +#endif
> +
>   	if (!init_pcmcia()) {
>   		/* XXX: shouldn't we re-enable irq here? */
>   		free_netdev(dev);
> @@ -583,6 +604,16 @@ static int init_pcmcia(void)
>   #endif
>   	u_long offset;
>   
> +	/* reset card (idea taken from CardReset by Artur Pogoda) */
> +	if (isa_type == ISA_TYPE_AG16) {
> +		u_char tmp = gayle.intreq;
> +
> +		gayle.intreq = 0xff;
> +		mdelay(1);
> +		gayle.intreq = tmp;
> +		mdelay(300);
> +	}
> +
>   	pcmcia_reset();
>   	pcmcia_program_voltage(PCMCIA_0V);
>   	pcmcia_access_speed(PCMCIA_SPEED_250NS);
> @@ -616,4 +647,42 @@ static int init_pcmcia(void)
>   	return 1;
>   }
>   
> +#if IS_ENABLED(CONFIG_PCMCIA)
> +static int pcmcia_is_16bit(void)
> +{
> +	u_char cftuple[258];
> +	int cftuple_len;
> +	tuple_t cftable_tuple;
> +	cistpl_cftable_entry_t cftable_entry;
> +
> +	cftuple_len = pcmcia_copy_tuple(CISTPL_CFTABLE_ENTRY, cftuple, 256);
> +	if (cftuple_len < 3)
> +		return 0;
> +#ifdef DEBUG
> +	else
> +		print_hex_dump(KERN_WARNING, "cftable: ", DUMP_PREFIX_NONE, 8,
> +			       sizeof(char), cftuple, cftuple_len, false);
> +#endif
> +
> +	/* build tuple_t struct and call pcmcia_parse_tuple */
> +	cftable_tuple.DesiredTuple = CISTPL_CFTABLE_ENTRY;
> +	cftable_tuple.TupleCode = CISTPL_CFTABLE_ENTRY;
> +	cftable_tuple.TupleData = &cftuple[2];
> +	cftable_tuple.TupleDataLen = cftuple_len - 2;
> +	cftable_tuple.TupleDataMax = cftuple_len - 2;
> +
> +	if (pcmcia_parse_tuple(&cftable_tuple, (cisparse_t *)&cftable_entry))
> +		return 0;
> +
> +#ifdef DEBUG
> +	pr_info("IO flags: %x\n", cftable_entry.io.flags);
> +#endif
> +
> +	if (cftable_entry.io.flags & CISTPL_IO_16BIT)
> +		return 1;
> +
> +	return 0;
> +}
> +#endif
> +
>   MODULE_LICENSE("GPL");
> 

Powered by blists - more mailing lists