iop_msg_pool

iop_msg_pool

[Geert Uytterhoeven]

arch/m68k/mac/iop.c has:

static struct iop_msg iop_msg_pool[NUM_IOP_MSGS];

As iop_msg_pool consumes 4928 bytes, it should be allocated dynamically,
especially for multi-platform kernels not running on Mac.

Furthermore:

/*
 * Initialize the IOPs, if present.
 */

void __init iop_init(void)
{
        int i;

        if (iop_scc_present) {
                printk("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
        }
        if (iop_ism_present) {
                printk("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
                iop_start(iop_base[IOP_NUM_ISM]);
                iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
        }

        /* Make the whole pool available and empty the queues */

        for (i = 0 ; i < NUM_IOP_MSGS ; i++) {
                iop_msg_pool[i].status = IOP_MSGSTATUS_UNUSED;
        }

        for (i = 0 ; i < NUM_IOP_CHAN ; i++) {
                iop_send_queue[IOP_NUM_SCC][i] = NULL;
                iop_send_queue[IOP_NUM_ISM][i] = NULL;
                iop_listeners[IOP_NUM_SCC][i].devname = NULL;
                iop_listeners[IOP_NUM_SCC][i].handler = NULL;
                iop_listeners[IOP_NUM_ISM][i].devname = NULL;
                iop_listeners[IOP_NUM_ISM][i].handler = NULL;
        }
}

However, despite the comment at the top, all of the above is done
unconditionally?
Do all Macs use the IOP functionality?

P.S. Before anyone mentions zorro_autocon: yes, it's on my list ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: iop_msg_pool

[Finn Thain]

On Wed, 26 Jun 2013, Geert Uytterhoeven wrote:

> arch/m68k/mac/iop.c has:
> 
> static struct iop_msg iop_msg_pool[NUM_IOP_MSGS];
> 
> As iop_msg_pool consumes 4928 bytes, it should be allocated dynamically, 
> especially for multi-platform kernels not running on Mac.

I agree. Though I don't care much for multi-platform kernels: bloat in 
portable code is easily excused when most users of that code run it on 
recent hardware. We don't have that excuse.

> 
> Furthermore:
> 
> /*
>  * Initialize the IOPs, if present.
>  */
> 
> void __init iop_init(void)
> {
>         int i;
> 
>         if (iop_scc_present) {
>                 printk("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
>         }
>         if (iop_ism_present) {
>                 printk("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
>                 iop_start(iop_base[IOP_NUM_ISM]);
>                 iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
>         }

Maybe use kcalloc() to allocate iop_msg_pool, iop_send_queue and 
iop_listeners and remove all of the following initialisation. I don't see 
a convenient way to call kfree() though.

> 
>         /* Make the whole pool available and empty the queues */
> 
>         for (i = 0 ; i < NUM_IOP_MSGS ; i++) {
>                 iop_msg_pool[i].status = IOP_MSGSTATUS_UNUSED;
>         }
> 
>         for (i = 0 ; i < NUM_IOP_CHAN ; i++) {
>                 iop_send_queue[IOP_NUM_SCC][i] = NULL;
>                 iop_send_queue[IOP_NUM_ISM][i] = NULL;
>                 iop_listeners[IOP_NUM_SCC][i].devname = NULL;
>                 iop_listeners[IOP_NUM_SCC][i].handler = NULL;
>                 iop_listeners[IOP_NUM_ISM][i].devname = NULL;
>                 iop_listeners[IOP_NUM_ISM][i].handler = NULL;
>         }
> }
> 
> However, despite the comment at the top, all of the above is done
> unconditionally?

Seems so. And it is pointless AFAICS with grep.

> Do all Macs use the IOP functionality?

No. iop_scc_present and iop_ism_present are accurate.

So far, the only driver to make use of this functionality is 
drivers/macintosh/adb-iop.c:

int adb_iop_probe(void)
{
        if (!iop_ism_present) return -ENODEV;
        return 0;
}

Finn

> 
> P.S. Before anyone mentions zorro_autocon: yes, it's on my list ;-)
> 
> Gr{oetje,eeting}s,
> 
>                         Geert

Re: iop_msg_pool

[Geert Uytterhoeven]

On Wed, Jun 26, 2013 at 12:48 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
>>         if (iop_scc_present) {
>>                 printk("IOP: detected SCC IOP at %p\n", iop_base[IOP_NUM_SCC]);
>>         }
>>         if (iop_ism_present) {
>>                 printk("IOP: detected ISM IOP at %p\n", iop_base[IOP_NUM_ISM]);
>>                 iop_start(iop_base[IOP_NUM_ISM]);
>>                 iop_alive(iop_base[IOP_NUM_ISM]); /* clears the alive flag */
>>         }
>
> Maybe use kcalloc() to allocate iop_msg_pool, iop_send_queue and
> iop_listeners and remove all of the following initialisation. I don't see
> a convenient way to call kfree() though.

I don't think not calling kfree() is an issue here. The code can't be
unloaded anyway.

>>
>>         /* Make the whole pool available and empty the queues */
>>
>>         for (i = 0 ; i < NUM_IOP_MSGS ; i++) {
>>                 iop_msg_pool[i].status = IOP_MSGSTATUS_UNUSED;
>>         }
>>
>>         for (i = 0 ; i < NUM_IOP_CHAN ; i++) {
>>                 iop_send_queue[IOP_NUM_SCC][i] = NULL;
>>                 iop_send_queue[IOP_NUM_ISM][i] = NULL;
>>                 iop_listeners[IOP_NUM_SCC][i].devname = NULL;
>>                 iop_listeners[IOP_NUM_SCC][i].handler = NULL;
>>                 iop_listeners[IOP_NUM_ISM][i].devname = NULL;
>>                 iop_listeners[IOP_NUM_ISM][i].handler = NULL;
>>         }
>> }
>>
>> However, despite the comment at the top, all of the above is done
>> unconditionally?
>
> Seems so. And it is pointless AFAICS with grep.
>
>> Do all Macs use the IOP functionality?
>
> No. iop_scc_present and iop_ism_present are accurate.

Thanks! So we can just do nothing and return if both of them are unset.

Are iop_scc_present and iop_sim_present mutually exclusive?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: iop_msg_pool

[Finn Thain]

On Wed, 26 Jun 2013, Geert Uytterhoeven wrote:

> Are iop_scc_present and iop_ism_present mutually exclusive?

Both are true or both are false, assuming the existing values in 
mac_data_table[]. But looking at iop_preinit(), I'd guess that the idea 
was to have the ability to disable either the SCC or the ADB channel for a 
particular model for development purposes. So I'd treat them as 
independent.

Finn

Re: iop_msg_pool

[Geert Uytterhoeven]

Hi Finn,

On Wed, Jun 26, 2013 at 12:48 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
>> As iop_msg_pool consumes 4928 bytes, it should be allocated dynamically,
>> especially for multi-platform kernels not running on Mac.
>
> I agree. Though I don't care much for multi-platform kernels: bloat in
> portable code is easily excused when most users of that code run it on
> recent hardware. We don't have that excuse.
>
> Maybe use kcalloc() to allocate iop_msg_pool, iop_send_queue and
> iop_listeners and remove all of the following initialisation. I don't see
> a convenient way to call kfree() though.

At this time, slab hasn't been initialized yet, so kzalloc() is out of
the question.
As iop_init() is called even before paging_init(), the bootmem allocator is also
not available yet.

I'm wondering if the call to iop_init() can be moved to mac_platform_init(),
where we can allocate memory?
The only callers into iop are adb-iop, which definitely runs later, and
mac_init_IRQ(), which calls iop_register_interrupts().
The latter may be an issue: can we receive iop interrupts immediately after
iop_register_interrupts()? If yes, this may call into an uninitialized
iop module.
Perhaps the call to iop_register_interrupts() can be done from iop_init()
instead?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: iop_msg_pool

[Finn Thain]

On Fri, 28 Jun 2013, Geert Uytterhoeven wrote:

> Hi Finn,
> 
> On Wed, Jun 26, 2013 at 12:48 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
> >>
> >> As iop_msg_pool consumes 4928 bytes, it should be allocated 
> >> dynamically, especially for multi-platform kernels not running on 
> >> Mac.
> >
> > I agree. Though I don't care much for multi-platform kernels: bloat in 
> > portable code is easily excused when most users of that code run it on 
> > recent hardware. We don't have that excuse.
> >
> > Maybe use kcalloc() to allocate iop_msg_pool, iop_send_queue and 
> > iop_listeners and remove all of the following initialisation. I don't 
> > see a convenient way to call kfree() though.
> 
> At this time, slab hasn't been initialized yet, so kzalloc() is out of 
> the question. As iop_init() is called even before paging_init(), the 
> bootmem allocator is also not available yet.
> 
> I'm wondering if the call to iop_init() can be moved to 
> mac_platform_init(), where we can allocate memory?

Yes, but it would have to be called before mac_platform_init() registers 
the SWIM device. Preferably with a comment to that effect.

The devices attached to the IOPs are SWIM, SCC and ADB. iop_preinit() 
places the SCC channel in bypass mode, so we don't need iop_init() to use 
the SCC. ADB doesn't start until device_initcall(adb_init).

> The only callers into iop are adb-iop, which definitely runs later, and 
> mac_init_IRQ(), which calls iop_register_interrupts(). The latter may be 
> an issue: can we receive iop interrupts immediately after 
> iop_register_interrupts()? If yes, this may call into an uninitialized 
> iop module.

That could be a problem if we get the sequence wrong.

> Perhaps the call to iop_register_interrupts() can be done from 
> iop_init() instead?

Yes, I think so. The VIA initialisation disables the IOP interrupt sources 
(IRQ_VIA1_2 and IRQ_VIA2_0). After that, we call via_register_interrupts() 
and later iop_register_interrupts(), and then call iop_init(). After that 
the ADB driver can start exchanging messages between CPU and the IOP. The 
sequence is important, but you can delay iop_register_interrupts() until 
iop_init().

So I would move the contents of iop_register_interrupts() to the beginning 
of iop_init(), after you allocate messaging memory but before iop_start() 
is called. I'd eliminate iop_register_interrupts() entirely (see iop.c, 
mac_iop.h, macints.c).

The following code in macints.c strikes me as slightly misleading.

173	if (oss_present)
174		oss_register_interrupts();
175	else
176		via_register_interrupts();
177	if (psc_present)
178		psc_register_interrupts();
179	if (baboon_present)
180		baboon_register_interrupts();
181	iop_register_interrupts();

OSS, VIA, PSC and Baboon handlers all dispatch interrupts to drivers that 
register them, whereas the IOP handler doesn't do that. It is actually a 
driver, not an interrupt controller. It uses VIA interrupts and provides 
messaging services to other drivers. So eliminating 
iop_register_interrupts() could improve clarity.

Finn

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Re: iop_msg_pool

[Geert Uytterhoeven]

On Sat, Jun 29, 2013 at 6:47 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> On Fri, 28 Jun 2013, Geert Uytterhoeven wrote:
>> On Wed, Jun 26, 2013 at 12:48 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
>> >> As iop_msg_pool consumes 4928 bytes, it should be allocated
>> >> dynamically, especially for multi-platform kernels not running on
>> >> Mac.
>> >
>> > I agree. Though I don't care much for multi-platform kernels: bloat in
>> > portable code is easily excused when most users of that code run it on
>> > recent hardware. We don't have that excuse.
>> >
>> > Maybe use kcalloc() to allocate iop_msg_pool, iop_send_queue and
>> > iop_listeners and remove all of the following initialisation. I don't
>> > see a convenient way to call kfree() though.
>>
>> At this time, slab hasn't been initialized yet, so kzalloc() is out of
>> the question. As iop_init() is called even before paging_init(), the
>> bootmem allocator is also not available yet.
>>
>> I'm wondering if the call to iop_init() can be moved to
>> mac_platform_init(), where we can allocate memory?
>
> Yes, but it would have to be called before mac_platform_init() registers
> the SWIM device. Preferably with a comment to that effect.
>
> The devices attached to the IOPs are SWIM, SCC and ADB. iop_preinit()
> places the SCC channel in bypass mode, so we don't need iop_init() to use
> the SCC. ADB doesn't start until device_initcall(adb_init).
>
>> The only callers into iop are adb-iop, which definitely runs later, and
>> mac_init_IRQ(), which calls iop_register_interrupts(). The latter may be
>> an issue: can we receive iop interrupts immediately after
>> iop_register_interrupts()? If yes, this may call into an uninitialized
>> iop module.
>
> That could be a problem if we get the sequence wrong.
>
>> Perhaps the call to iop_register_interrupts() can be done from
>> iop_init() instead?
>
> Yes, I think so. The VIA initialisation disables the IOP interrupt sources
> (IRQ_VIA1_2 and IRQ_VIA2_0). After that, we call via_register_interrupts()
> and later iop_register_interrupts(), and then call iop_init(). After that
> the ADB driver can start exchanging messages between CPU and the IOP. The
> sequence is important, but you can delay iop_register_interrupts() until
> iop_init().
>
> So I would move the contents of iop_register_interrupts() to the beginning
> of iop_init(), after you allocate messaging memory but before iop_start()
> is called. I'd eliminate iop_register_interrupts() entirely (see iop.c,
> mac_iop.h, macints.c).
>
> The following code in macints.c strikes me as slightly misleading.
>
> 173     if (oss_present)
> 174             oss_register_interrupts();
> 175     else
> 176             via_register_interrupts();
> 177     if (psc_present)
> 178             psc_register_interrupts();
> 179     if (baboon_present)
> 180             baboon_register_interrupts();
> 181     iop_register_interrupts();
>
> OSS, VIA, PSC and Baboon handlers all dispatch interrupts to drivers that
> register them, whereas the IOP handler doesn't do that. It is actually a
> driver, not an interrupt controller. It uses VIA interrupts and provides
> messaging services to other drivers. So eliminating
> iop_register_interrupts() could improve clarity.

Thans for your explanation! I'll do it that way.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: iop_msg_pool

[Brad Boyer]

On Sat, Jun 29, 2013 at 02:47:34PM +1000, Finn Thain wrote:
> The devices attached to the IOPs are SWIM, SCC and ADB. iop_preinit() 
> places the SCC channel in bypass mode, so we don't need iop_init() to use 
> the SCC. ADB doesn't start until device_initcall(adb_init).

It could become more complicated if we started using the SCC IOP and
wanted to be able to do serial console, although I suppose we could
use bypass mode until the real driver is loaded later and switch at
runtime. It would make some things messy but might still be simpler
than complicating this part of the code.

	Brad Boyer
	flar@allandria.com

Re: iop_msg_pool

[Brad Boyer]

On Wed, Jun 26, 2013 at 08:48:14PM +1000, Finn Thain wrote:
> On Wed, 26 Jun 2013, Geert Uytterhoeven wrote:
> > Do all Macs use the IOP functionality?
> 
> No. iop_scc_present and iop_ism_present are accurate.

In fact, very few Macs use the IOP chips. Only three models used
them, and they are some of the rarest models on top of that. These
chips can be found on the IIfx, the Quadra 900, and the Quadra 950.

> So far, the only driver to make use of this functionality is 
> drivers/macintosh/adb-iop.c:
> 
> int adb_iop_probe(void)
> {
>         if (!iop_ism_present) return -ENODEV;
>         return 0;
> }

We did have an IOP SWIM driver at one point, but it got removed
from the tree. It couldn't read or write blocks, so it wasn't
actually useful. However, it did correctly detect drives and I
think it had disk insert detection and eject as well.

The block data transfer to and from the IOP is messy, and I
couldn't get it working when I tried to fix it back then.

Like the SCC IOP, it is possible to use bypass mode to access
the SWIM chip directly. However, there is no way to have ADB
if the ISM IOP is in bypass mode.

	Brad Boyer
	flar@allandria.com

Re: iop_msg_pool

[Finn Thain]

On Sun, 30 Jun 2013, Brad Boyer wrote:

> On Sat, Jun 29, 2013 at 02:47:34PM +1000, Finn Thain wrote:
> > The devices attached to the IOPs are SWIM, SCC and ADB. iop_preinit() 
> > places the SCC channel in bypass mode, so we don't need iop_init() to 
> > use the SCC. ADB doesn't start until device_initcall(adb_init).
> 
> It could become more complicated if we started using the SCC IOP and 
> wanted to be able to do serial console, although I suppose we could use 
> bypass mode until the real driver is loaded later and switch at runtime. 
> It would make some things messy but might still be simpler than 
> complicating this part of the code.

I can't imagine anyone writing an IOP-based SCC driver unless they wanted 
to implement DMA and LocalTalk support. The present code should work fine 
with the pmac_zilog serial driver. It should also work with EARLY_CONSOLE 
if MacOS is configured to put the IOPs into bypass (aka "compatible") mode 
before Penguin boots Linux.

Finn

> 
> 	Brad Boyer
> 	flar@allandria.com
> 

Re: iop_msg_pool

[Brad Boyer]

On Mon, Jul 01, 2013 at 05:54:41PM +1000, Finn Thain wrote:
> 
> On Sun, 30 Jun 2013, Brad Boyer wrote:
> > 
> > It could become more complicated if we started using the SCC IOP and 
> > wanted to be able to do serial console, although I suppose we could use 
> > bypass mode until the real driver is loaded later and switch at runtime. 
> > It would make some things messy but might still be simpler than 
> > complicating this part of the code.
> 
> I can't imagine anyone writing an IOP-based SCC driver unless they wanted 
> to implement DMA and LocalTalk support. The present code should work fine 
> with the pmac_zilog serial driver. It should also work with EARLY_CONSOLE 
> if MacOS is configured to put the IOPs into bypass (aka "compatible") mode 
> before Penguin boots Linux.

Well, it would be less overhead even for regular serial usage. You wouldn't
need to interrupt the processor as often. But it's probably not worth the
effort. Getting LocalTalk working would be nearly impossible without some
hardware assist such as an IOP (or the PSC-DMA on the models that have it).

	Brad Boyer
	flar@allandria.com