[PATCH v2] can: grcan: Add device driver for GRCAN and GRHCAN cores
Wolfgang Grandegger
wg at grandegger.com
Tue Oct 30 20:29:00 EST 2012
Hi Andreas,
sorry for late answer. Other duties are keeping me busy. Your v3
triggered a closer look...
On 10/24/2012 03:31 PM, Andreas Larsson wrote:
> On 10/23/2012 06:26 PM, Wolfgang Grandegger wrote:
>> Hi,
>>
>> before I have a closer look I have some general questions, especially
>> about the heavy usage of SysFS for configuring the IP core module.
>> Generally, we are not allowed to use SysFS for CAN device configuration.
>>
>> - Why may the user want to configure the resources on a per device base?
>
> The GRCAN core supports selecting between two different hardware
> interfaces. The parameters output0, output1 and selection configures
> these interfaces and selects which one to use. This can not be done in
> general. For output0 and output1 what value means what depends on what
> hardware is connected to the core. If a board has many GRCAN cores they
> might need different settings for these variables. In addition,
> switching between the two hardware interfaces is something that might be
> wanted to be done at runtime.
>
> For the others module level configuration would work fine.
OK.
>
>> - Aren't there good default values which work just fine for 99% of the
>> users?
>
> For output0, output1 and selection, the answer is no due to the reasons
> pointed out above. For the bpr and buffer sizes, that is probably
true.
OK, reducing the number of SysFS files to a really useful number (for
end users) would be nice.
...
>>> +What: /sys/class/net/<iface>/grcan/rxmask
>>> +Date: October 2012
>>> +KernelVersion: 3.8
>>> +Contact: Andreas Larsson<andreas at gaisler.com>
>>> +Description:
>>> + Configures the rxmask of the hardware filter. Received messages
>>> + for which ((message_id ^ rxcode) & rxmask) != 0 holds will be
>>> + filtered out in harware. Possible values in [0, 0x1fffffff].
>>> + The default value is set by the module parameter rxmask and can
>>> + be read at /sys/module/grcan/parameters/rxmask.
>>
>> Hardware filters should definitely not be defined via SysFS. We do not
>> have an interface yet, mainly because it does not fit into a multi user
>> concept. Anyway, we need such an interface *sooner* than later. Needs
>> some further thoughts.
>
> OK, I'll get rid of that then and wait for such an interface in the
> future. It would be a pity if hardware filtering, that is a feature that
> would probably be used on an embedded system without multiple users,
> could never be realized because of the socket interface being a multi
> user concept. If root is the only one that can set it up it should be
> fine in my opinion. Nevertheless, I totally agree that a well defined
> API to enable it would be much nicer than going through SysFS.
Yes, we definitely need a generic interface to support hardware filters.
I put it on top of my Linux-CAN-TODO-List.
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/grcan.txt
>>> @@ -0,0 +1,27 @@
>>> +CAN controller for Aeroflex Gaisler GRCAN and GRHCAN.
>>> +
>>> +The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL
>>> IP core
>>> +library.
>>> +
>>> +Note: These properties are built from the AMBA plug&play in a Leon
>>> SPARC
>>> +system (the ordinary environment for GRCAN and GRHCAN). There are no
>>> bsp
>>> +files for sparc.
>>> +
>>> +Required properties:
>>> +
>>> +- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or
>>> "01_034"
>>
>> A name should be "vendor,device", e.g. gaisler,grcan. It's also unusual
>> to add release numbers. Also, you do not distinguish between the devices
>> above. Then, just use one name and the others are compatible to that one
>> (even if they are named differently). Well, I realized that the device
>> tree police is less strict than it was 1..2 years ago... anyway, please
>> have a look to the many examples around, also for CAN.
>
> As I stated in the file, there are no bsp files for sparc. The device
> trees are generated by a boot loader or a prom. For a leon sparc system
> the boot loader gets information from the hardware, the AMBA plug&play,
> and generates the device trees accordingly.
Ah, the system uses good old classic OpenFirmware. Sorry, I missed that.
This makes a lot of my questions obsolete.
More information about the devicetree-discuss
mailing list