Re: linux-next: Tree for Oct 11 (mmc)

Re: linux-next: Tree for Oct 11 (mmc)

[Randy Dunlap]

On 10/11/11 02:11, Stephen Rothwell wrote:
> Hi all,
> 
> The linux-next tree is now available from
> git://github.com/sfrothwell/linux-next.git as a temporary measure while
> the kernel.org servers are unavailable.
> 
> It may also turn up on git.kernel.org (depending on the mirroring).  The
> patch set is still absent, however.
> 
> Changes since 20111007:


When CONFIG_BLOCK is not enabled:

In file included from next-2011-1011/drivers/mmc/card/sdio_uart.c:43:0:
next-2011-1011/include/linux/mmc/card.h:175:12: error: 'DISK_NAME_LEN' undeclared here (not in a function)

Deleting the #include <linux/mmc/card.h> fixes the sdio_uart.c build.
However, the same problem occurs in mmc/core/core.c:

In file included from next-2011-1011/drivers/mmc/core/core.c:30:0:
next-2011-1011/include/linux/mmc/card.h:175:12: error: 'DISK_NAME_LEN' undeclared here (not in a function)

Should mmc/core/ depend on BLOCK?  or should it just be made
to build even when BLOCK is not enabled?

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[Andrei Warkentin]

Hi Randy,

----- Original Message -----
> From: "Randy Dunlap" <rdunlap@xenotime.net>
> To: "Stephen Rothwell" <sfr@canb.auug.org.au>
> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
> <cjb@laptop.org>
> Sent: Tuesday, October 11, 2011 2:49:39 PM
> Subject: Re: linux-next: Tree for Oct 11 (mmc)
> 
> On 10/11/11 02:11, Stephen Rothwell wrote:
> > Hi all,
> > 
> > The linux-next tree is now available from
> > git://github.com/sfrothwell/linux-next.git as a temporary measure
> > while
> > the kernel.org servers are unavailable.
> > 
> > It may also turn up on git.kernel.org (depending on the mirroring).
> >  The
> > patch set is still absent, however.
> > 
> > Changes since 20111007:
> 
> 
> When CONFIG_BLOCK is not enabled:
> 
> In file included from
> next-2011-1011/drivers/mmc/card/sdio_uart.c:43:0:
> next-2011-1011/include/linux/mmc/card.h:175:12: error:
> 'DISK_NAME_LEN' undeclared here (not in a function)
> 
> Deleting the #include <linux/mmc/card.h> fixes the sdio_uart.c build.
> However, the same problem occurs in mmc/core/core.c:
> 

Because linux/genhd is now included, oops. I'm pretty positive this is due to the "mmc : general purpose partition support" patch pulled recently. I am adding NamJae, who was the author.

> In file included from next-2011-1011/drivers/mmc/core/core.c:30:0:
> next-2011-1011/include/linux/mmc/card.h:175:12: error:
> 'DISK_NAME_LEN' undeclared here (not in a function)
> 
> Should mmc/core/ depend on BLOCK?  or should it just be made
> to build even when BLOCK is not enabled?
> 

I don't think there should be a direct dependency on BLOCK. I have two suggestions -
1) Have our own define similar to (and in fact smaller):
   linux/genhd.h:#define DISK_NAME_LEN                     32
2) Put the MMC physical partition code under an #ifdef CONFIG_BLOCK, which is a reasonable
   proposition, given that there wouldn't be any need to parse physical partition info if
   it would never be consumed by the MMC block driver.

Thoughts?

A

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[Randy Dunlap]

On 10/11/11 12:31, Andrei Warkentin wrote:
> Hi Randy,
> 
> ----- Original Message -----
>> From: "Randy Dunlap" <rdunlap@xenotime.net>
>> To: "Stephen Rothwell" <sfr@canb.auug.org.au>
>> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
>> <cjb@laptop.org>
>> Sent: Tuesday, October 11, 2011 2:49:39 PM
>> Subject: Re: linux-next: Tree for Oct 11 (mmc)
>>
>> On 10/11/11 02:11, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> The linux-next tree is now available from
>>> git://github.com/sfrothwell/linux-next.git as a temporary measure
>>> while
>>> the kernel.org servers are unavailable.
>>>
>>> It may also turn up on git.kernel.org (depending on the mirroring).
>>>  The
>>> patch set is still absent, however.
>>>
>>> Changes since 20111007:
>>
>>
>> When CONFIG_BLOCK is not enabled:
>>
>> In file included from
>> next-2011-1011/drivers/mmc/card/sdio_uart.c:43:0:
>> next-2011-1011/include/linux/mmc/card.h:175:12: error:
>> 'DISK_NAME_LEN' undeclared here (not in a function)
>>
>> Deleting the #include <linux/mmc/card.h> fixes the sdio_uart.c build.
>> However, the same problem occurs in mmc/core/core.c:
>>
> 
> Because linux/genhd is now included, oops. I'm pretty positive this is due to the "mmc : general purpose partition support" patch pulled recently. I am adding NamJae, who was the author.
> 
>> In file included from next-2011-1011/drivers/mmc/core/core.c:30:0:
>> next-2011-1011/include/linux/mmc/card.h:175:12: error:
>> 'DISK_NAME_LEN' undeclared here (not in a function)
>>
>> Should mmc/core/ depend on BLOCK?  or should it just be made
>> to build even when BLOCK is not enabled?
>>
> 
> I don't think there should be a direct dependency on BLOCK. I have two suggestions -
> 1) Have our own define similar to (and in fact smaller):
>    linux/genhd.h:#define DISK_NAME_LEN                     32
> 2) Put the MMC physical partition code under an #ifdef CONFIG_BLOCK, which is a reasonable
>    proposition, given that there wouldn't be any need to parse physical partition info if
>    it would never be consumed by the MMC block driver.
> 
> Thoughts?

Agreed on part 2).  Do part 1) if it is required, but it's usually better
not to duplicate constants or structs etc.
IOW, can DISK_NAME_LEN in linux/genhd.h be exposed even when CONFIG_BLOCK
is not enabled?



-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[NamJae Jeon]

2011/10/12 Randy Dunlap <rdunlap@xenotime.net>:
> On 10/11/11 12:31, Andrei Warkentin wrote:
>> Hi Randy,
>>
>> ----- Original Message -----
>>> From: "Randy Dunlap" <rdunlap@xenotime.net>
>>> To: "Stephen Rothwell" <sfr@canb.auug.org.au>
>>> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
>>> <cjb@laptop.org>
>>> Sent: Tuesday, October 11, 2011 2:49:39 PM
>>> Subject: Re: linux-next: Tree for Oct 11 (mmc)
>>>
>>> On 10/11/11 02:11, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> The linux-next tree is now available from
>>>> git://github.com/sfrothwell/linux-next.git as a temporary measure
>>>> while
>>>> the kernel.org servers are unavailable.
>>>>
>>>> It may also turn up on git.kernel.org (depending on the mirroring).
>>>>  The
>>>> patch set is still absent, however.
>>>>
>>>> Changes since 20111007:
>>>
>>>
>>> When CONFIG_BLOCK is not enabled:
>>>
>>> In file included from
>>> next-2011-1011/drivers/mmc/card/sdio_uart.c:43:0:
>>> next-2011-1011/include/linux/mmc/card.h:175:12: error:
>>> 'DISK_NAME_LEN' undeclared here (not in a function)
>>>
>>> Deleting the #include <linux/mmc/card.h> fixes the sdio_uart.c build.
>>> However, the same problem occurs in mmc/core/core.c:
>>>
>>
>> Because linux/genhd is now included, oops. I'm pretty positive this is due to the "mmc : general purpose partition support" patch pulled recently. I am adding NamJae, who was the author.
>>
>>> In file included from next-2011-1011/drivers/mmc/core/core.c:30:0:
>>> next-2011-1011/include/linux/mmc/card.h:175:12: error:
>>> 'DISK_NAME_LEN' undeclared here (not in a function)
>>>
>>> Should mmc/core/ depend on BLOCK?  or should it just be made
>>> to build even when BLOCK is not enabled?
>>>
>>
>> I don't think there should be a direct dependency on BLOCK. I have two suggestions -
>> 1) Have our own define similar to (and in fact smaller):
>>    linux/genhd.h:#define DISK_NAME_LEN                     32
>> 2) Put the MMC physical partition code under an #ifdef CONFIG_BLOCK, which is a reasonable
>>    proposition, given that there wouldn't be any need to parse physical partition info if
>>    it would never be consumed by the MMC block driver.
>>
>> Thoughts?
>
> Agreed on part 2).  Do part 1) if it is required, but it's usually better
> not to duplicate constants or structs etc.
> IOW, can DISK_NAME_LEN in linux/genhd.h be exposed even when CONFIG_BLOCK
> is not enabled?

Hi Randy, Andrei.

I suggest third option for this.
As you know, MMC like ATA Driver and SCSI Driver etc.. can not enable
without CONFIG_BLOCK
So I think that mmc should be depended from CONFIG_BLOCK like other
block device driver.
see the their Kconfig. How do you think ?
--------------------------------------------------------------------------------------------------------------------------------
menuconfig ATA
        tristate "Serial ATA and Parallel ATA drivers"
        depends on HAS_IOMEM
        depends on BLOCK
        depends on !(M32R || M68K) || BROKEN


config SCSI
        tristate "SCSI device support"
        depends on BLOCK
        select SCSI_DMA if HAS_DMA
        ---help---
---------------------------------------------------------------------------------------------------------------------

>
>
> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[Andrei Warkentin]

----- Original Message -----
> From: "NamJae Jeon" <linkinjeon@gmail.com>
> To: "Randy Dunlap" <rdunlap@xenotime.net>, "Andrei Warkentin" <awarkentin@vmware.com>
> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>
> Sent: Tuesday, October 11, 2011 7:20:48 PM
> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))
> 
> Hi Randy, Andrei.
> 
> I suggest third option for this.
> As you know, MMC like ATA Driver and SCSI Driver etc.. can not enable
> without CONFIG_BLOCK
> So I think that mmc should be depended from CONFIG_BLOCK like other
> block device driver.
> see the their Kconfig. How do you think ?

MMC core doesn't not imply MMC_BLOCK. You could well use SDIO devices via MMC without any flash storage whatsoever.
What I want to say is that MMC_BLOCK already depends on BLOCK. MMC, however, has no such functional dependence, as it
just (effectively) provides bus and device enumeration. So I think the better solution is wrapping all MMC partition
code within mmc/core/mmc.c and card.h with CONFIG_BLOCK.

A

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[NamJae Jeon]

2011/10/12 Andrei Warkentin <awarkentin@vmware.com>:
> ----- Original Message -----
>> From: "NamJae Jeon" <linkinjeon@gmail.com>
>> To: "Randy Dunlap" <rdunlap@xenotime.net>, "Andrei Warkentin" <awarkentin@vmware.com>
>> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
>> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>
>> Sent: Tuesday, October 11, 2011 7:20:48 PM
>> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))
>>
>> Hi Randy, Andrei.
>>
>> I suggest third option for this.
>> As you know, MMC like ATA Driver and SCSI Driver etc.. can not enable
>> without CONFIG_BLOCK
>> So I think that mmc should be depended from CONFIG_BLOCK like other
>> block device driver.
>> see the their Kconfig. How do you think ?
>
> MMC core doesn't not imply MMC_BLOCK. You could well use SDIO devices via MMC without any flash storage whatsoever.
> What I want to say is that MMC_BLOCK already depends on BLOCK. MMC, however, has no such functional dependence, as it
> just (effectively) provides bus and device enumeration. So I think the better solution is wrapping all MMC partition
> code within mmc/core/mmc.c and card.h with CONFIG_BLOCK.
yes, you're right, I found it after sending mail. If so, should I wrap
CONFIG_MMC_BLOCK instead of CONFIG_MMC ? After I add CONFIG_MMC_BLOCK
in core/mmc.c, card.h, I can see compile is okay.
Thanks.
>
> A
>

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[Andrei Warkentin]

Hi,

----- Original Message -----
> From: "NamJae Jeon" <linkinjeon@gmail.com>
> To: "Andrei Warkentin" <awarkentin@vmware.com>
> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "Randy Dunlap" <rdunlap@xenotime.net>
> Sent: Tuesday, October 11, 2011 8:16:51 PM
> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))
> 
> 2011/10/12 Andrei Warkentin <awarkentin@vmware.com>:
> > ----- Original Message -----
> >> From: "NamJae Jeon" <linkinjeon@gmail.com>
> >> To: "Randy Dunlap" <rdunlap@xenotime.net>, "Andrei Warkentin"
> >> <awarkentin@vmware.com>
> >> Cc: linux-next@vger.kernel.org, "LKML"
> >> <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris
> >> Ball"
> >> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>
> >> Sent: Tuesday, October 11, 2011 7:20:48 PM
> >> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was:
> >> linux-next: Tree for Oct 11 (mmc))
> >>
> >> Hi Randy, Andrei.
> >>
> >> I suggest third option for this.
> >> As you know, MMC like ATA Driver and SCSI Driver etc.. can not
> >> enable
> >> without CONFIG_BLOCK
> >> So I think that mmc should be depended from CONFIG_BLOCK like
> >> other
> >> block device driver.
> >> see the their Kconfig. How do you think ?
> >
> > MMC core doesn't not imply MMC_BLOCK. You could well use SDIO
> > devices via MMC without any flash storage whatsoever.
> > What I want to say is that MMC_BLOCK already depends on BLOCK. MMC,
> > however, has no such functional dependence, as it
> > just (effectively) provides bus and device enumeration. So I think
> > the better solution is wrapping all MMC partition
> > code within mmc/core/mmc.c and card.h with CONFIG_BLOCK.
> yes, you're right, I found it after sending mail. If so, should I
> wrap
> CONFIG_MMC_BLOCK instead of CONFIG_MMC ? After I add CONFIG_MMC_BLOCK
> in core/mmc.c, card.h, I can see compile is okay.
> Thanks.
> >

I am not sure if it should be CONFIG_MMC_BLOCK or CONFIG_BLOCK. After all, the
code you're wrapping doesn't really depend on CONFIG_MMC_BLOCK, it gets consumed by it, and
it depends (in using that one define) only on CONFIG_BLOCK. Maybe I'm overthinking it
and the code should just define it's own MAX_MMC_PART_NAME to be like 10 or something.

A

Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))

[NamJae Jeon]

2011/10/12 Andrei Warkentin <awarkentin@vmware.com>:
> Hi,
>
> ----- Original Message -----
>> From: "NamJae Jeon" <linkinjeon@gmail.com>
>> To: "Andrei Warkentin" <awarkentin@vmware.com>
>> Cc: linux-next@vger.kernel.org, "LKML" <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris Ball"
>> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>, "Randy Dunlap" <rdunlap@xenotime.net>
>> Sent: Tuesday, October 11, 2011 8:16:51 PM
>> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was: linux-next: Tree for Oct 11 (mmc))
>>
>> 2011/10/12 Andrei Warkentin <awarkentin@vmware.com>:
>> > ----- Original Message -----
>> >> From: "NamJae Jeon" <linkinjeon@gmail.com>
>> >> To: "Randy Dunlap" <rdunlap@xenotime.net>, "Andrei Warkentin"
>> >> <awarkentin@vmware.com>
>> >> Cc: linux-next@vger.kernel.org, "LKML"
>> >> <linux-kernel@vger.kernel.org>, linux-mmc@vger.kernel.org, "Chris
>> >> Ball"
>> >> <cjb@laptop.org>, "Stephen Rothwell" <sfr@canb.auug.org.au>
>> >> Sent: Tuesday, October 11, 2011 7:20:48 PM
>> >> Subject: Re: mmc core broken dependency on CONFIG_BLOCK (Was:
>> >> linux-next: Tree for Oct 11 (mmc))
>> >>
>> >> Hi Randy, Andrei.
>> >>
>> >> I suggest third option for this.
>> >> As you know, MMC like ATA Driver and SCSI Driver etc.. can not
>> >> enable
>> >> without CONFIG_BLOCK
>> >> So I think that mmc should be depended from CONFIG_BLOCK like
>> >> other
>> >> block device driver.
>> >> see the their Kconfig. How do you think ?
>> >
>> > MMC core doesn't not imply MMC_BLOCK. You could well use SDIO
>> > devices via MMC without any flash storage whatsoever.
>> > What I want to say is that MMC_BLOCK already depends on BLOCK. MMC,
>> > however, has no such functional dependence, as it
>> > just (effectively) provides bus and device enumeration. So I think
>> > the better solution is wrapping all MMC partition
>> > code within mmc/core/mmc.c and card.h with CONFIG_BLOCK.
>> yes, you're right, I found it after sending mail. If so, should I
>> wrap
>> CONFIG_MMC_BLOCK instead of CONFIG_MMC ? After I add CONFIG_MMC_BLOCK
>> in core/mmc.c, card.h, I can see compile is okay.
>> Thanks.
>> >
>
> I am not sure if it should be CONFIG_MMC_BLOCK or CONFIG_BLOCK. After all, the
> code you're wrapping doesn't really depend on CONFIG_MMC_BLOCK, it gets consumed by it, and
> it depends (in using that one define) only on CONFIG_BLOCK. Maybe I'm overthinking it
> and the code should just define it's own MAX_MMC_PART_NAME to be like 10 or something.
yes, I agree your opinion,  If we define it is easy to solve.
I will send new patch for it today.
Thanks.
>
> A
>