Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Sep 2019 16:46:16 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r351870 - stable/12/sys/arm/ti/am335x
Message-ID:  <201909051646.x85GkGbT087344@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Thu Sep  5 16:46:16 2019
New Revision: 351870
URL: https://svnweb.freebsd.org/changeset/base/351870

Log:
  MFC r350838, r350840-r350841, r350849, r350879
  
  r350838:
  Switch the am335x_pmic driver to using iicdev_readfrom/writeto.
  
  PR:		239697
  Submitted by:	Chuhong Yuan
  
  r350840:
  Garbage collect the no-longer-necessary MAX_IIC_DATA_SIZE (there is not a
  buffer allocated at that fixed size anymore).
  
  r350841:
  When responding to an interrupt in the am335x_pmic driver, use a taskqueue
  thread to do the work that involves i2c IO, which sleeps while the IO is
  in progress.
  
  r350849:
  Remove use of intr_config_hook from the am335x_pmic and tda19988 drivers.
  Long ago this was needed, but now low-level i2c controller drivers cleverly
  defer attachment of the bus until interrupts are enabled (if they require
  interrupts to function), so that every i2c slave device doesn't have to.
  
  r350879:
  Revert r350841.  I didn't realize that on this chip, reading the interrupt
  status register clears pending interrupts.  By moving that code out of the
  interrupt handler into a taskqueue task, I effectively created an interrupt
  storm by returning from the handler with the interrupt source still active.
  
  We'll have to find a different solution for this driver's need to sleep
  in an ithread context.

Modified:
  stable/12/sys/arm/ti/am335x/am335x_pmic.c
  stable/12/sys/arm/ti/am335x/tda19988.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/arm/ti/am335x/am335x_pmic.c
==============================================================================
--- stable/12/sys/arm/ti/am335x/am335x_pmic.c	Thu Sep  5 16:37:10 2019	(r351869)
+++ stable/12/sys/arm/ti/am335x/am335x_pmic.c	Thu Sep  5 16:46:16 2019	(r351870)
@@ -56,13 +56,9 @@ __FBSDID("$FreeBSD$");
 
 #include "iicbus_if.h"
 
-#define MAX_IIC_DATA_SIZE	2
-
-
 struct am335x_pmic_softc {
 	device_t		sc_dev;
 	uint32_t		sc_addr;
-	struct intr_config_hook enum_hook;
 	struct resource		*sc_irq_res;
 	void			*sc_intrhand;
 };
@@ -79,30 +75,13 @@ static void am335x_pmic_shutdown(void *, int);
 static int
 am335x_pmic_read(device_t dev, uint8_t addr, uint8_t *data, uint8_t size)
 {
-	struct am335x_pmic_softc *sc = device_get_softc(dev);
-	struct iic_msg msg[] = {
-		{ sc->sc_addr, IIC_M_WR, 1, &addr },
-		{ sc->sc_addr, IIC_M_RD, size, data },
-	};
-	return (iicbus_transfer(dev, msg, 2));
+	return (iicdev_readfrom(dev, addr, data, size, IIC_INTRWAIT));
 }
 
 static int
 am335x_pmic_write(device_t dev, uint8_t address, uint8_t *data, uint8_t size)
 {
-	uint8_t buffer[MAX_IIC_DATA_SIZE + 1];
-	struct am335x_pmic_softc *sc = device_get_softc(dev);
-	struct iic_msg msg[] = {
-		{ sc->sc_addr, IIC_M_WR, size + 1, buffer },
-	};
-
-	if (size > MAX_IIC_DATA_SIZE)
-		return (ENOMEM);
-
-	buffer[0] = address;
-	memcpy(buffer + 1, data, size);
-
-	return (iicbus_transfer(dev, msg, 1));
+	return (iicdev_writeto(dev, address, data, size, IIC_INTRWAIT));
 }
 
 static void
@@ -220,10 +199,9 @@ am335x_pmic_setvo(device_t dev, uint8_t vo)
 }
 
 static void
-am335x_pmic_start(void *xdev)
+am335x_pmic_start(struct am335x_pmic_softc *sc)
 {
-	struct am335x_pmic_softc *sc;
-	device_t dev = (device_t)xdev;
+	device_t dev;
 	struct tps65217_status_reg status_reg;
 	struct tps65217_chipid_reg chipid_reg;
 	uint8_t reg, vo;
@@ -231,8 +209,7 @@ am335x_pmic_start(void *xdev)
 	char pwr[4][11] = {"Battery", "USB", "AC", "USB and AC"};
 	int rv;
 
-	sc = device_get_softc(dev);
-
+	dev = sc->sc_dev;
 	am335x_pmic_read(dev, TPS65217_CHIPID_REG, (uint8_t *)&chipid_reg, 1);
 	switch (chipid_reg.chip) {
 		case TPS65217A:
@@ -275,8 +252,6 @@ am335x_pmic_start(void *xdev)
 	EVENTHANDLER_REGISTER(shutdown_final, am335x_pmic_shutdown, dev,
 	    SHUTDOWN_PRI_LAST);
 
-	config_intrhook_disestablish(&sc->enum_hook);
-
 	/* Unmask all interrupts and clear pending status */
 	reg = 0;
 	am335x_pmic_write(dev, TPS65217_INT_REG, &reg, 1);
@@ -308,11 +283,7 @@ am335x_pmic_attach(device_t dev)
 		/* return (ENXIO); */
 	}
 
-	sc->enum_hook.ich_func = am335x_pmic_start;
-	sc->enum_hook.ich_arg = dev;
-
-	if (config_intrhook_establish(&sc->enum_hook) != 0)
-		return (ENOMEM);
+	am335x_pmic_start(sc);
 
 	return (0);
 }

Modified: stable/12/sys/arm/ti/am335x/tda19988.c
==============================================================================
--- stable/12/sys/arm/ti/am335x/tda19988.c	Thu Sep  5 16:37:10 2019	(r351869)
+++ stable/12/sys/arm/ti/am335x/tda19988.c	Thu Sep  5 16:46:16 2019	(r351870)
@@ -242,7 +242,6 @@ struct tda19988_softc {
 	uint32_t		sc_addr;
 	uint32_t		sc_cec_addr;
 	uint16_t		sc_version;
-	struct intr_config_hook enum_hook;
 	int			sc_current_page;
 	uint8_t			*sc_edid;
 	uint32_t		sc_edid_len;
@@ -644,15 +643,14 @@ done:
 }
 
 static void
-tda19988_start(void *xdev)
+tda19988_start(struct tda19988_softc *sc)
 {
-	struct tda19988_softc *sc;
-	device_t dev = (device_t)xdev;
+	device_t dev;
 	uint8_t data;
 	uint16_t version;
 
-	sc = device_get_softc(dev);
-
+	dev = sc->sc_dev;
+	
 	tda19988_cec_write(sc, TDA_CEC_ENAMODS, ENAMODS_RXSENS | ENAMODS_HDMI);
 	DELAY(1000);
 	tda19988_cec_read(sc, 0xfe, &data);
@@ -698,7 +696,7 @@ tda19988_start(void *xdev)
 			break;
 		default:
 			device_printf(dev, "Unknown device: %04x\n", sc->sc_version);
-			goto done;
+			return;
 	}
 
 	tda19988_reg_write(sc, TDA_DDC_CTRL, DDC_ENABLE);
@@ -709,16 +707,13 @@ tda19988_start(void *xdev)
 
 	if (tda19988_read_edid(sc) < 0) {
 		device_printf(dev, "failed to read EDID\n");
-		goto done;
+		return;
 	}
 
 	/* Default values for RGB 4:4:4 mapping */
 	tda19988_reg_write(sc, TDA_VIP_CNTRL_0, 0x23);
 	tda19988_reg_write(sc, TDA_VIP_CNTRL_1, 0x01);
 	tda19988_reg_write(sc, TDA_VIP_CNTRL_2, 0x45);
-
-done:
-	config_intrhook_disestablish(&sc->enum_hook);
 }
 
 static int
@@ -737,14 +732,10 @@ tda19988_attach(device_t dev)
 
 	device_set_desc(dev, "NXP TDA19988 HDMI transmitter");
 
-	sc->enum_hook.ich_func = tda19988_start;
-	sc->enum_hook.ich_arg = dev;
-
-	if (config_intrhook_establish(&sc->enum_hook) != 0)
-		return (ENOMEM);
-
 	node = ofw_bus_get_node(dev);
 	OF_device_register_xref(OF_xref_from_node(node), dev);
+
+	tda19988_start(sc);
 
 	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201909051646.x85GkGbT087344>