[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b0438a630811240912v16db8ef8v660cb844979fe912@mail.gmail.com>
Date: Mon, 24 Nov 2008 18:12:47 +0100
From: "Miguel Ángel Álvarez" <gotzoncabanes@...il.com>
To: "Krzysztof Halasa" <khc@...waw.pl>
Cc: netdev@...r.kernel.org, linux-arm-kernel@...ts.arm.linux.org.uk
Subject: Re: ixp4xx_hss MAX_CHAN_DEVICES
Hi
2008/11/24 Krzysztof Halasa <khc@...waw.pl>:
> "Miguel Ángel Álvarez" <gotzoncabanes@...il.com> writes:
>
>> +++ linux-2.6.26.7/drivers/net/wan/ixp4xx_hss.c 2008-11-24
>
>> +struct physical_modes {
>> + u8 id;
>> + unsigned int frame_size;
>> + unsigned int clock_rate;
>> + u32 clock_config;
>> +};
>
> I think the clock rate shouldn't be a part of the fixed port config.
> I don't know much about 4E1 configs, but in the usual case (single
> physical port connected directly to the HSS) you can use many clock
> and frame size configurations.
>
I am not quite sure about this. If we do not set the clock_rate as
part of the "physical_mode"... where should it be set? As a separate
variable? I do not have problems with that neither.
> IOW I think physical mode = only signal/timeslot routing topology.
> The platform code could also optionally check if the specified clock
> type and rate is valid.
>
Is this what you are stating? a separate variable for clock_rate in
the platform structure?
>> +static const struct physical_modes phmodes[] = {
>> + {IXP4XX_HSS_T1, 193, 1544000, CLK42X_SPEED_1544KHZ},
>> + {IXP4XX_HSS_E1, 256, 2048000, CLK42X_SPEED_2048KHZ},
>> + {IXP4XX_HSS_2_E1, 512, 4096000, CLK42X_SPEED_4096KHZ},
>> + {IXP4XX_HSS_4_E1, 1024, 8192000, CLK42X_SPEED_8192KHZ},
>> + {0, 256, 2048000, CLK42X_SPEED_2048KHZ}
>> +};
>
> If we need different internal clock rates (most framers/multiplexers
> provide their own, don't they?) then I guess we need to calculate the
> required settings ourselves. I don't exactly know how do they
> calculate the register but it should be easy to find out. There was no
> need yet.
>
I agree... I think the precalculated values are enough for now.
>> +#define IXP4XX_HSS_T1 0x10
>> +#define IXP4XX_HSS_E1 0x11
>
> T1 and E1 are the same WRT hardware connection to the port.
>
I have done the difference for the clock_rates and the needing of a
framing bit in the case of T1.
>> - How should all these changes interact with show_frame_size and
>> set_frame_size?
>
> Perhaps the platform code should initialize the value and then check
> changes for validity. Only matters with G.704 framers I think.
> Otherwise the user should be able to select anything.
>
Yes... but the dynamic allocation makes it more difficult.
>> - I am a bit lost with MAX_CHANNELS and MAX_CHAN_DEVICES... Which are
>> the differences between both?
>
> MAX_CHANNELS = (currenly) 32 = max for single channel (32 E1
> channels).
> MAX_CHAN_DEVICES = limit of /dev/hssXchY nodes (for one port).
>
OK. Thanks... However... shouldn't they be the same value... I mean...
It could be possible to have a device for each channel in the frame,
not? So, for example if I had 128 channels (4*E1), it could be
required to deal with 128 devices, not? (In channelized mode).
>> - As MAX_CHANNELS and MAX_CHAN_DEVICES should not be set by defines, I
>> am going to alloc memory for chan_devices... I am going to do it in
>> init_one... Is it OK?.
>
> MAX_CHANNELS, I think so.
> MAX_CHAN_DEVICES isn't directly related.
>
I do not understand why are not they directly related.
>> My first intention for all this is to set HSS into MVIP mode and have
>> 4 HDLC channels in a packetized mode.
>> - How could I set this MVIP mode?
>> - How could I interface with generic HDLC so that hss_hdlc_xmit sends
>> the data for each stream to a different FIFO?
>
> The HSS driver should initialize MVIP based on platform's
> physical_config value, then it would have to register 4 HDLC devices.
>
Sure... Let me re-do the question. Do you know how MVIP is set if I
detect it is needed? And... good idea with the registering of four
devices.
> The HDLC part (not the channelized one) should be quite trivial.
> I think we now need the HSS HDLC driver upstream, will look at this.
> --
Sorry... I have not understood this.
Miguel Ángel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists