Re: port-arm/49621: Add detach function to if_cpsw

[Robert Sprowson <webpages%sprow.co.uk@localhost>]

The following reply was made to PR port-arm/49621; it has been noted by GNATS.

From: Robert Sprowson <webpages%sprow.co.uk@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: port-arm-maintainer%netbsd.org@localhost,
	gnats-admin%netbsd.org@localhost,
	netbsd-bugs%netbsd.org@localhost
Subject: Re: port-arm/49621: Add detach function to if_cpsw
Date: Sun, 01 Feb 2015 19:00:58 +0000 (GMT)

 > Doesn't it need to do at least a bus_space_unmap(), call cpsw_stop()
 > and do all the other interface cleanup that the other drivers do
 > when detaching?
 
 You're quite right, I'd only been considering the equivalents of
 malloc/free, but cpsw_match has other (non allocating) side effects. This
 updated patch should address those too, inspired by the i82577 driver:
 
 --- if_cpsw_1.6.c	2014-10-31 17:12:48 +0000
 +++ if_cpsw_new.c	2015-02-01 18:41:06 +0000
 @@ -114,16 +114,18 @@
  struct cpsw_softc {
  	device_t sc_dev;
  	bus_space_tag_t sc_bst;
  	bus_space_handle_t sc_bsh;
 +	bus_size_t sc_bss;
  	bus_dma_tag_t sc_bdt;
  	bus_space_handle_t sc_bsh_txdescs;
  	bus_space_handle_t sc_bsh_rxdescs;
  	bus_addr_t sc_txdescs_pa;
  	bus_addr_t sc_rxdescs_pa;
  	struct ethercom sc_ec;
  	struct mii_data sc_mii;
  	bool sc_phy_has_1000t;
 +	bool sc_attached;
  	callout_t sc_tick_ch;
  	void *sc_ih;
  	struct cpsw_ring_data *sc_rdp;
  	volatile u_int sc_txnext;
 @@ -144,8 +146,9 @@
  };
  
  static int cpsw_match(device_t, cfdata_t, void *);
  static void cpsw_attach(device_t, device_t, void *);
 +static int cpsw_detach(device_t, int);
  
  static void cpsw_start(struct ifnet *);
  static int cpsw_ioctl(struct ifnet *, u_long, void *);
  static void cpsw_watchdog(struct ifnet *);
 @@ -169,9 +172,9 @@
  
  static int cpsw_ale_update_addresses(struct cpsw_softc *, int purge);
  
  CFATTACH_DECL_NEW(cpsw, sizeof(struct cpsw_softc),
 -    cpsw_match, cpsw_attach, NULL, NULL);
 +    cpsw_match, cpsw_attach, cpsw_detach, NULL);
  
  #undef KERNHIST
  #include <sys/kernhist.h>
  KERNHIST_DEFINE(cpswhist);
 @@ -335,8 +338,57 @@
  	}
  	return false;
  }
  
 +static int
 +cpsw_detach(device_t self, int flags)
 +{
 +	struct cpsw_softc * const sc = device_private(self);
 +	struct ifnet *ifp = &sc->sc_ec.ec_if;
 +	u_int i;
 +
 +	/* Succeed now if there's no work to do. */
 +	if (!sc->sc_attached)
 +		return (0);
 +
 +	/* Stop the interface. Callouts are stopped in it. */
 +	cpsw_stop(ifp, 1);
 +
 +	/* Destroy our callout. */
 +	callout_destroy(&sc->sc_tick_ch);
 +
 +	/* Delete all media. */
 +	ifmedia_delete_instance(&sc->sc_mii.mii_media, IFM_INST_ANY);
 +
 +	ether_ifdetach(ifp);
 +	if_detach(ifp);
 +
 +	/* Let go of the interrupts */
 +	intr_disestablish(sc->sc_rxthih);
 +	intr_disestablish(sc->sc_rxih);
 +	intr_disestablish(sc->sc_txih);
 +	intr_disestablish(sc->sc_miscih);
 +
 +	/* Free the packet padding buffer */
 +	kmem_free(sc->sc_txpad, ETHER_MIN_LEN);
 +	bus_dmamap_destroy(sc->sc_bdt, sc->sc_txpad_dm);
 +
 +	/* Destroy all the descriptors */
 +	for (i = 0; i < CPSW_NTXDESCS; i++)
 +		bus_dmamap_destroy(sc->sc_bdt, sc->sc_rdp->tx_dm[i]);
 +	for (i = 0; i < CPSW_NRXDESCS; i++)
 +		bus_dmamap_destroy(sc->sc_bdt, sc->sc_rdp->rx_dm[i]);
 +	kmem_free(sc->sc_rdp, sizeof(*sc->sc_rdp));
 +
 +	/* Unmap */
 +	bus_space_unmap(sc->sc_bst, sc->sc_bsh, sc->sc_bss);
 +
 +	sc->sc_attached = false;
 +	aprint_normal_dev("detached CPSW driver\n");
 +
 +	return 0;
 +}
 +
  static void
  cpsw_attach(device_t parent, device_t self, void *aux)
  {
  	struct obio_attach_args * const oa = aux;
 @@ -397,8 +449,9 @@
  	sc->sc_miscih = intr_establish(oa->obio_intrbase + CPSW_INTROFF_MISC,
  	    IPL_VM, IST_LEVEL, cpsw_miscintr, sc);
  
  	sc->sc_bst = oa->obio_iot;
 +	sc->sc_bss = oa->obio_size;
  	sc->sc_bdt = oa->obio_dmat;
  
  	error = bus_space_map(sc->sc_bst, oa->obio_addr, oa->obio_size, 0,
  	    &sc->sc_bsh);
 @@ -514,8 +567,11 @@
  
  	if_attach(ifp);
  	ether_ifattach(ifp, sc->sc_enaddr);
  
 +	/* The attach is successful. */
 +	sc->sc_attached = true;
 +
  	return;
  }
  
  static void