From: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public•gmane.org>
To: Subhasish Ghosh
<subhasish-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org,
Netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org,
nsekhar-l0cyMroinI0@public•gmane.org,
open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
CAN NETWORK DRIVERS
<socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public•gmane.org>,
Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public•gmane.org>,
m-watkins-l0cyMroinI0@public•gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public•gmane.org
Subject: Re: [PATCH v4 1/1] can: add pruss CAN driver.
Date: Wed, 27 Apr 2011 15:28:50 +0200 [thread overview]
Message-ID: <4DB81A12.1000006@grandegger.com> (raw)
In-Reply-To: <46D523E49EFF489F9B088AE7B9CD7286@subhasishg>
On 04/27/2011 03:08 PM, Subhasish Ghosh wrote:
>>
>> - Use just *one* value per sysfs file
>
> SG - I felt adding entry for each mbx_id will clutter the sysfs.
> Is it ok to do that.
No, see:
http://lxr.linux.no/#linux+v2.6.38/Documentation/filesystems/sysfs.txt#L56
>>>> +static u32 pruss_intc_init[19][3] = {
>>>> + {PRUSS_INTC_POLARITY0, PRU_INTC_REGMAP_MASK, 0xFFFFFFFF},
>>>> + {PRUSS_INTC_POLARITY1, PRU_INTC_REGMAP_MASK, 0xFFFFFFFF},
>>>> + {PRUSS_INTC_TYPE0, PRU_INTC_REGMAP_MASK, 0x1C000000},
>>>> + {PRUSS_INTC_TYPE1, PRU_INTC_REGMAP_MASK, 0},
>>>> + {PRUSS_INTC_GLBLEN, 0, 1},
>>>> + {PRUSS_INTC_HOSTMAP0, PRU_INTC_REGMAP_MASK, 0x03020100},
>>>> + {PRUSS_INTC_HOSTMAP1, PRU_INTC_REGMAP_MASK, 0x07060504},
>>>> + {PRUSS_INTC_HOSTMAP2, PRU_INTC_REGMAP_MASK, 0x0000908},
>>>> + {PRUSS_INTC_CHANMAP0, PRU_INTC_REGMAP_MASK, 0},
>>>> + {PRUSS_INTC_CHANMAP8, PRU_INTC_REGMAP_MASK, 0x00020200},
>>>> + {PRUSS_INTC_STATIDXCLR, 0, 32},
>>>> + {PRUSS_INTC_STATIDXCLR, 0, 19},
>>>> + {PRUSS_INTC_ENIDXSET, 0, 19},
>>>> + {PRUSS_INTC_STATIDXCLR, 0, 18},
>>>> + {PRUSS_INTC_ENIDXSET, 0, 18},
>>>> + {PRUSS_INTC_STATIDXCLR, 0, 34},
>>>> + {PRUSS_INTC_ENIDXSET, 0, 34},
>>>> + {PRUSS_INTC_ENIDXSET, 0, 32},
>>>> + {PRUSS_INTC_HOSTINTEN, 0, 5}
>>>
>>> please add ","
>>
>> Also a struct to describe each entry would improve readability.
>> Then you could also use ARRAY_SIZE.
>
> SG _ I could not follow this, are you recommending that I create a
> structure with three variables and then create
> an array for it.
> something like:
>
> const static struct [] = {
> {
> unsigned int reg_base;
> unsigned int reg_mask;
> unsigned int reg_val;
> },
> ...
> };
Yes:
struct s_name {
unsigned int base;
unsigned int mask;
unsigned int val;
};
const static struct s_name array[] = {
...
};
>
>>>> + value = (PRUSS_CAN_GPIO_SETUP_DELAY *
>>>> + (priv->clk_freq_pru / 1000000) / 1000) /
>>>> + PRUSS_CAN_DELAY_LOOP_LENGTH;
>>
>> This calculation looks delicate. 64-bit math would be safer.
>
> SG - This one works fine. I am dividing it twice to avoid the problem.
Yes, but what if the frequency increases with the next generation of the
hardware?
Wolfgang.
next prev parent reply other threads:[~2011-04-27 13:28 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1303474267-6344-1-git-send-email-subhasish@mistralsolutions.com>
2011-04-22 12:11 ` [PATCH v4 1/1] can: add pruss CAN driver Subhasish Ghosh
[not found] ` <1303474267-6344-2-git-send-email-subhasish-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org>
2011-04-22 15:50 ` Marc Kleine-Budde
[not found] ` <4DB1A3B7.7060300-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2011-04-25 20:06 ` Wolfgang Grandegger
[not found] ` <4DB5D452.9050500-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-04-27 13:08 ` Subhasish Ghosh
2011-04-27 13:21 ` Marc Kleine-Budde
2011-04-27 13:25 ` Arnd Bergmann
[not found] ` <201104271525.28512.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-04 7:13 ` Subhasish Ghosh
2011-05-04 13:11 ` Arnd Bergmann
[not found] ` <201105041511.54095.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-04 14:33 ` Wolfgang Grandegger
[not found] ` <4DC163D7.9010309-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-05-04 14:48 ` Arnd Bergmann
[not found] ` <201105041648.37199.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-04 16:00 ` Wolfgang Grandegger
[not found] ` <4DC17831.3070801-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-05-10 10:11 ` Subhasish Ghosh
2011-05-10 10:27 ` Alan Cox
[not found] ` <20110510112734.54160824-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2011-05-10 12:21 ` Subhasish Ghosh
2011-05-11 21:31 ` Arnd Bergmann
[not found] ` <201105112331.47954.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-11 21:44 ` Arnd Bergmann
[not found] ` <201105112344.44171.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-11 22:39 ` Marc Kleine-Budde
2011-05-11 22:56 ` Alan Cox
[not found] ` <20110511235652.7ccddb3b-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2011-05-12 3:03 ` can: hardware vs. software filter Kurt Van Dijck
2011-05-12 7:13 ` [PATCH v4 1/1] can: add pruss CAN driver Wolfgang Grandegger
[not found] ` <4DCB88A4.2010901-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-05-12 10:58 ` Kurt Van Dijck
2011-05-12 12:54 ` Arnd Bergmann
[not found] ` <201105121454.47781.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-12 13:04 ` Marc Kleine-Budde
2011-05-12 14:41 ` Oliver Hartkopp
[not found] ` <4DCBF1B6.6000104-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
2011-05-22 10:30 ` Arnd Bergmann
[not found] ` <201105221230.56243.arnd-r2nGTMty4D4@public.gmane.org>
2011-05-23 6:21 ` Oliver Hartkopp
[not found] ` <4DD9FCFC.10803-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
2011-05-23 8:23 ` Marc Kleine-Budde
2011-05-27 8:31 ` Wolfgang Grandegger
2011-05-12 7:04 ` Wolfgang Grandegger
2011-05-04 15:57 ` Kurt Van Dijck
2011-05-04 16:09 ` Wolfgang Grandegger
[not found] ` <4DC17A31.8070409@grandegger.com>
[not found] ` <4DC17A31.8070409-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-05-04 20:55 ` Oliver Hartkopp
2011-04-27 13:28 ` Wolfgang Grandegger [this message]
[not found] ` <4DB81A12.1000006-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
2011-04-27 13:34 ` Wolfgang Grandegger
2011-04-24 11:13 ` Marc Kleine-Budde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DB81A12.1000006@grandegger.com \
--to=wg-5yr1bzd7o62+xt7jha+gda@public$(echo .)gmane.org \
--cc=Netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public$(echo .)gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=m-watkins-l0cyMroinI0@public$(echo .)gmane.org \
--cc=mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public$(echo .)gmane.org \
--cc=nsekhar-l0cyMroinI0@public$(echo .)gmane.org \
--cc=sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org \
--cc=socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public$(echo .)gmane.org \
--cc=subhasish-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox