Hello,
I wonder if this patch should be further split.
To my taste this should be at least:
- Allocate McBSP devices dynamically,
- add 34xx support
maybe more, see below.
> diff --git a/arch/arm/mach-omap1/mcbsp.c b/arch/arm/mach-omap1/mcbsp.c
> index afb5789..7de7c69 100644
> --- a/arch/arm/mach-omap1/mcbsp.c
> +++ b/arch/arm/mach-omap1/mcbsp.c
> @@ -103,30 +103,6 @@ static inline void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
> { }
> #endif
>
> -static int omap1_mcbsp_check(unsigned int id)
> -{
> - /* REVISIT: Check correctly for number of registered McBSPs */
> - if (cpu_is_omap730()) {
> - if (id > OMAP_MAX_MCBSP_COUNT - 2) {
> - printk(KERN_ERR "OMAP-McBSP: McBSP%d doesn't exist\n",
> - id + 1);
> - return -ENODEV;
> - }
> - return 0;
> - }
> -
> - if (cpu_is_omap15xx() || cpu_is_omap16xx()) {
> - if (id > OMAP_MAX_MCBSP_COUNT - 1) {
> - printk(KERN_ERR "OMAP-McBSP: McBSP%d doesn't exist\n",
> - id + 1);
> - return -ENODEV;
> - }
> - return 0;
> - }
> -
> - return -ENODEV;
> -}
> -
> static void omap1_mcbsp_request(unsigned int id)
> {
> /*
> @@ -151,7 +127,6 @@ static void omap1_mcbsp_free(unsigned int id)
> }
>
> static struct omap_mcbsp_ops omap1_mcbsp_ops = {
> - .check = omap1_mcbsp_check,
Why can this function go away? IMHO it should either go away in a
separate commit or be extended to be more general to apply for the 34xx
case, too. (Same for omap2_mcbsp_check.)
> @@ -185,7 +226,7 @@ static struct omap_mcbsp_platform_data omap34xx_mcbsp_pdata[] = {
> #define OMAP34XX_MCBSP_PDATA_SZ 0
> #endif
>
> -int __init omap2_mcbsp_init(void)
> +static int __init omap2_mcbsp_init(void)
> {
> int i;
>
OK, this is trivial, but if you ask me, at least note it in the commit
message.
> @@ -196,9 +237,18 @@ int __init omap2_mcbsp_init(void)
> }
>
> if (cpu_is_omap24xx())
> + omap_mcbsp_count = OMAP24XX_MCBSP_PDATA_SZ;
> + if (cpu_is_omap34xx())
> + omap_mcbsp_count = OMAP34XX_MCBSP_PDATA_SZ;
> +
> + mcbsp_ptr = kzalloc(omap_mcbsp_count * sizeof(struct omap_mcbsp *),
> + GFP_KERNEL);
> + if (!mcbsp_ptr)
> + return -ENOMEM;
> +
> + if (cpu_is_omap24xx())
> omap_mcbsp_register_board_cfg(omap24xx_mcbsp_pdata,
> OMAP24XX_MCBSP_PDATA_SZ);
> -
> if (cpu_is_omap34xx())
> omap_mcbsp_register_board_cfg(omap34xx_mcbsp_pdata,
> OMAP34XX_MCBSP_PDATA_SZ);
Hhm, it looks there is already some 34xx support in the file. Should
the commit log be more detailed?
(Note, I didn't check the rest of the patch.)
Best regards
Uwe
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
