svn commit: r290373 - head/sys/dev/ofw

[Andreas Tobler]

On 05.11.15 22:47, Nathan Whitehorn wrote:
> On 11/05/15 13:24, Andreas Tobler wrote:
>> Hi Nathan, Warner,
>>
>> first, I had the feeling that I have to provide a fast solution which
>> makes PowerMacs usable again. I am aware that the committed version
>> doesn't win a prize.
>
> Thank you!
>
>> Also, I didn't know that we have AIM & FDT, Nathan which one?
>
> PS3 and POWER8 systems are AIM+FDT. Neither of these have I2C busses in
> their FDTs, so the impact is low for now, but it makes me a little nervous.

Ah, maybe I have luck to convince my son to handover the PS3 to me. He 
has a PS4 too...

About the POWER8, well, far away from my budget.....

>> On 05.11.15 21:26, Warner Losh wrote:
>>> I'd suggested that this be driven off a global quirk like
>>>
>>> u_int fdt_quirks;
>>> #define FDT_QUIRK_8BIT_IIC_ADDR 1
>>> ...
>>
>> In openfirm.h?
>
> I'd really prefer an explicit platform check in ofw_iicbus.c for this by
> looking at the compatible property of the root node. If this ever comes
> up on another system, we can modify it, but I think it won't.

Attached a working proposal.
It does work with the Apple 8-bit I2C addresses and the fan regulation 
works.

But I need a kind soul to test this on an arm board with Linux based FDT 
I2C devices which have 7-bit addresses.

Ian?

Thanks in advance,
Andreas

>>> if (fdt_quirks & FDT_QUIRK_8BIT_IIC_ADDR)
>>> dinfo->opd_dinfo.addr = paddr;
>>> else
>>> dinfo->opd_dinfo.addr = paddr << 1;
>>>
>>> And the platform code, whatever that means, would set it when it "knows"
>>> this is the case.
>>
>> If I get that right, I'd have to set the fdt_quirk in each I2C parent,
>> like kiic.c and smu.c?
>>
>>> On Thu, Nov 5, 2015 at 12:56 PM, Nathan Whitehorn
>>> <nwhitehorn at freebsd.org <mailto:nwhitehorn at freebsd.org>> wrote:
>>>
>>>      I'm not sure this is the best way to do this: we have AIM systems
>>>      that use FDT, for example. Can we make it a quirk in the host-bus
>>>      driver? Or do a run-time check to see if the root node has MacRISC
>>>      in its compatible property?
>>
>> The 'compatible property' approach would be limited/isolated to one
>> file (ofw_iicbus.c) in comparison to the quirk approach where I'd have
>> to adapt at least (known now) four files.
>>
>> I can go either way, just my thoughts.


-------------- next part --------------
Index: sys/dev/ofw/ofw_iicbus.c
===================================================================
--- sys/dev/ofw/ofw_iicbus.c	(revision 290373)
+++ sys/dev/ofw/ofw_iicbus.c	(working copy)
@@ -101,9 +101,13 @@
 {
 	struct iicbus_softc *sc = IICBUS_SOFTC(dev);
 	struct ofw_iicbus_devinfo *dinfo;
-	phandle_t child, node;
+	phandle_t child, node, root;
 	pcell_t freq, paddr;
 	device_t childdev;
+	ssize_t compatlen;
+	char compat[255];
+	char *curstr;
+	u_int iic_addr_8bit = 0;
 
 	sc->dev = dev;
 	mtx_init(&sc->lock, "iicbus", NULL, MTX_DEF);
@@ -125,6 +129,21 @@
 	bus_enumerate_hinted_children(dev);
 
 	/*
+	 * Check if we're running on a PowerMac, needed for the I2C
+	 * address below.
+	 */
+	root = OF_peer(0);
+	compatlen = OF_getprop(root, "compatible", compat,
+				sizeof(compat));
+	if (compatlen != -1) {
+	    for (curstr = compat; curstr < compat + compatlen;
+		curstr += strlen(curstr) + 1) {
+		if (strncmp(curstr, "MacRISC", 7) == 0)
+		    iic_addr_8bit = 1;
+	    }
+	}
+
+	/*
 	 * Attach those children represented in the device tree.
 	 */
 	for (child = OF_child(node); child != 0; child = OF_peer(child)) {
@@ -153,11 +172,11 @@
 		 * Linux FDT data contains 7-bit values, so shift them up to
 		 * 8-bit format.
 		 */
-#ifdef AIM
-		dinfo->opd_dinfo.addr = paddr;
-#else
-		dinfo->opd_dinfo.addr = paddr << 1;
-#endif
+		if (iic_addr_8bit)
+		    dinfo->opd_dinfo.addr = paddr;
+		else
+		    dinfo->opd_dinfo.addr = paddr << 1;
+
 		if (ofw_bus_gen_setup_devinfo(&dinfo->opd_obdinfo, child) !=
 		    0) {
 			free(dinfo, M_DEVBUF);